* [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size @ 2022-03-18 17:46 Michael Niedermayer 2022-03-18 17:56 ` Andreas Rheinhardt 0 siblings, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2022-03-18 17:46 UTC (permalink / raw) To: FFmpeg development discussions and patches 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); -- 2.17.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size 2022-03-18 17:46 [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size Michael Niedermayer @ 2022-03-18 17:56 ` Andreas Rheinhardt 2022-03-19 17:38 ` Michael Niedermayer 0 siblings, 1 reply; 6+ messages in thread From: Andreas Rheinhardt @ 2022-03-18 17:56 UTC (permalink / raw) To: ffmpeg-devel 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/; also see https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-11-andreas.rheinhardt@gmail.com/ and https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-1-andreas.rheinhardt@gmail.com/ - Andreas _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size 2022-03-18 17:56 ` Andreas Rheinhardt @ 2022-03-19 17:38 ` Michael Niedermayer 2022-03-21 18:54 ` Michael Niedermayer 0 siblings, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2022-03-19 17:38 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2298 bytes --] 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/; > also see > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-11-andreas.rheinhardt@gmail.com/ > and > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-1-andreas.rheinhardt@gmail.com/ Why did you not apply your bugfixes ? I think you should apply them thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old. [-- 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size 2022-03-19 17:38 ` Michael Niedermayer @ 2022-03-21 18:54 ` Michael Niedermayer 2022-03-21 20:59 ` Andreas Rheinhardt 0 siblings, 1 reply; 6+ messages in thread From: Michael Niedermayer @ 2022-03-21 18:54 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size 2022-03-21 18:54 ` Michael Niedermayer @ 2022-03-21 20:59 ` Andreas Rheinhardt 2022-03-21 21:12 ` Michael Niedermayer 0 siblings, 1 reply; 6+ messages in thread From: Andreas Rheinhardt @ 2022-03-21 20:59 UTC (permalink / raw) To: ffmpeg-devel 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) > > >>> 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. > >>> 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 > > [...] > > _______________________________________________ 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size 2022-03-21 20:59 ` Andreas Rheinhardt @ 2022-03-21 21:12 ` Michael Niedermayer 0 siblings, 0 replies; 6+ messages in thread From: Michael Niedermayer @ 2022-03-21 21:12 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- 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". ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-21 21:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-18 17:46 [FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size Michael Niedermayer 2022-03-18 17:56 ` Andreas Rheinhardt 2022-03-19 17:38 ` Michael Niedermayer 2022-03-21 18:54 ` Michael Niedermayer 2022-03-21 20:59 ` Andreas Rheinhardt 2022-03-21 21:12 ` Michael Niedermayer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git