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

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/4] avcodec/evc_ps: Check ref_pic_num and sps_max_dec_pic_buffering_minus1
Date: Fri, 15 Sep 2023 17:01:51 +0200
Message-ID: <20230915150151.GV8640@pb2> (raw)
In-Reply-To: <55fe7d49-901c-5055-61e2-975599a903f8@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2947 bytes --]

On Fri, Sep 15, 2023 at 10:57:29AM -0300, James Almer wrote:
> On 9/15/2023 10:11 AM, Michael Niedermayer wrote:
> > Fixes: out of array write
> > 
> > Found-by: dongsookim@korea.ac.kr
> > Signed-off-by: Michael Niedermayer <michael@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.

will apply with these changes

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".