Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size

[Michael Niedermayer <michael@niedermayer.cc>]

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

On Mon, Mar 21, 2022 at 09:59:00PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Sat, Mar 19, 2022 at 06:38:08PM +0100, Michael Niedermayer wrote:
> >> On Fri, Mar 18, 2022 at 06:56:19PM +0100, Andreas Rheinhardt wrote:
> >>> Michael Niedermayer:
> >>>> Fixes: Out of array read
> >>>> Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
> >>>>
> >>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>>> ---
> >>>>  libavcodec/vp9_superframe_split_bsf.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
> >>>> index ed0444561a..481484a4f0 100644
> >>>> --- a/libavcodec/vp9_superframe_split_bsf.c
> >>>> +++ b/libavcodec/vp9_superframe_split_bsf.c
> >>>> @@ -51,7 +51,7 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
> >>>>              return ret;
> >>>>          in = s->buffer_pkt;
> >>>>  
> >>>> -        marker = in->data[in->size - 1];
> >>>> +        marker = in->size ? in->data[in->size - 1] : 0;
> >>>>          if ((marker & 0xe0) == 0xc0) {
> >>>>              int length_size = 1 + ((marker >> 3) & 0x3);
> >>>>              int   nb_frames = 1 + (marker & 0x7);
> >>>
> >>> There is a second place in this BSF where data might be read in the
> >>> absence of data, namely if one of the frames in a superframe have size
> >>> of zero (its attempted to read its profile; no actual read takes place
> >>> due to the checks of the get_bits API, but it is nevertheless invalid
> >>> data). See
> >>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-7-andreas.rheinhardt@gmail.com/;
> > 
> > The get bits API checks for NULL data, if data is not NULL it must be padded
> > even when size is 0.
> > Nothing against the 2nd check, but thats a seperate issue
> 
> I know that there is no invalid read (and said as much)

I read what you wrote to quick, i missed this


> 
> > 
> > 
> >>> also see
> >>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-11-andreas.rheinhardt@gmail.com/
> > 
> > please apply your bugfixes! especially if its about out or array accesses
> > 
> 
> Will do.

thx

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- 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".