[FFmpeg-devel] [PATCH 2/4] avcodec/evc_ps: Check ref_pic_num and sps_max_dec_pic_buffering_minus1

[James Almer]

On 9/15/2023 10:11 AM, Michael Niedermayer wrote:
> Fixes: out of array write
> 
> Found-by: dongsookim at korea.ac.kr
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>   libavcodec/evc_ps.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c
> index 7fe13fd32f0..96237ed2911 100644
> --- a/libavcodec/evc_ps.c
> +++ b/libavcodec/evc_ps.c
> @@ -22,12 +22,15 @@
>   #include "evc_ps.h"
>   
>   #define EXTENDED_SAR 255
> -
>   // @see ISO_IEC_23094-1 (7.3.7 Reference picture list structure syntax)
> -static int ref_pic_list_struct(GetBitContext *gb, RefPicListStruct *rpl)
> +static int ref_pic_list_struct(EVCParserSPS *sps, GetBitContext *gb, RefPicListStruct *rpl)
>   {
>       uint32_t delta_poc_st, strp_entry_sign_flag = 0;
>       rpl->ref_pic_num = get_ue_golomb_long(gb);
> +
> +    if ((unsigned)rpl->ref_pic_num  > sps->sps_max_dec_pic_buffering_minus1)
> +        return AVERROR_INVALIDDATA;
> +
>       if (rpl->ref_pic_num > 0) {
>           delta_poc_st = get_ue_golomb_long(gb);
>   
> @@ -251,6 +254,8 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
>           sps->max_num_tid0_ref_pics = get_ue_golomb_31(gb);
>       else {
>           sps->sps_max_dec_pic_buffering_minus1 = get_ue_golomb_long(gb);
> +        if ((unsigned)sps->sps_max_dec_pic_buffering_minus1 > 16 - 1)
> +            return AVERROR_INVALIDDATA;
>           sps->long_term_ref_pic_flag = get_bits1(gb);
>           sps->rpl1_same_as_rpl0_flag = get_bits1(gb);
>           sps->num_ref_pic_list_in_sps[0] = get_ue_golomb(gb);
> @@ -261,7 +266,7 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
>           }
>   
>           for (int i = 0; i < sps->num_ref_pic_list_in_sps[0]; ++i)
> -            ref_pic_list_struct(gb, &sps->rpls[0][i]);
> +            ref_pic_list_struct(sps, gb, &sps->rpls[0][i]);

Could check and propagate the error value here while at it.

>   
>           if (!sps->rpl1_same_as_rpl0_flag) {
>               sps->num_ref_pic_list_in_sps[1] = get_ue_golomb(gb);
> @@ -270,7 +275,7 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
>                   goto fail;
>               }
>               for (int i = 0; i < sps->num_ref_pic_list_in_sps[1]; ++i)
> -                ref_pic_list_struct(gb, &sps->rpls[1][i]);
> +                ref_pic_list_struct(sps, gb, &sps->rpls[1][i]);

Ditto.

>           }
>       }
>   

Should be ok.