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

[Michael Niedermayer <michael@niedermayer.cc>]

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

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


> > 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


> > and
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-1-andreas.rheinhardt@gmail.com/

thats now found by the fuzzer too, in 45722
if you dont apply your fix i will post a fix

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

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