diff mbox series

[FFmpeg-devel,2/4] tools/target_dec_fuzzer: Do not test AV_CODEC_FLAG2_FAST with AV_CODEC_ID_H264

Message ID 20200315212058.6324-2-michael@niedermayer.cc
State Accepted
Commit 05d364dcccb3703de3f299b6ebaa13021b12c061
Headers show
Series [FFmpeg-devel,1/4] avformat/asfdec_f: Fix overflow check in get_tag()
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Michael Niedermayer March 15, 2020, 9:20 p.m. UTC
This combination skips allocating large padding which can read out of array

Fixes: 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 tools/target_dec_fuzzer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Niedermayer May 27, 2020, 9:53 p.m. UTC | #1
On Sun, Mar 15, 2020 at 10:20:56PM +0100, Michael Niedermayer wrote:
> This combination skips allocating large padding which can read out of array
> 
> Fixes: 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  tools/target_dec_fuzzer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

[...]
Kieran Kunhya May 27, 2020, 10:08 p.m. UTC | #2
On Wed, 27 May 2020 at 22:53, Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sun, Mar 15, 2020 at 10:20:56PM +0100, Michael Niedermayer wrote:
> > This combination skips allocating large padding which can read out of
> array
> >
> > Fixes:
> 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360
> >
> > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  tools/target_dec_fuzzer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> will apply
>

Shouldn't there be warnings about FAST mode if it causes security issues?

Kieran
Jean-Baptiste Kempf May 28, 2020, 9:14 a.m. UTC | #3
On Thu, May 28, 2020, at 00:08, Kieran Kunhya wrote:
> On Wed, 27 May 2020 at 22:53, Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Sun, Mar 15, 2020 at 10:20:56PM +0100, Michael Niedermayer wrote:
> > > This combination skips allocating large padding which can read out of
> > array
> > >
> > > Fixes:
> > 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360
> > >
> > > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  tools/target_dec_fuzzer.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > will apply
> >
> 
> Shouldn't there be warnings about FAST mode if it causes security issues?

I agree here.
Either we fix FAST mode, or it must come with important warnings.
Michael Niedermayer May 28, 2020, 12:23 p.m. UTC | #4
On Thu, May 28, 2020 at 11:14:15AM +0200, Jean-Baptiste Kempf wrote:
> On Thu, May 28, 2020, at 00:08, Kieran Kunhya wrote:
> > On Wed, 27 May 2020 at 22:53, Michael Niedermayer <michael@niedermayer.cc>
> > wrote:
> > 
> > > On Sun, Mar 15, 2020 at 10:20:56PM +0100, Michael Niedermayer wrote:
> > > > This combination skips allocating large padding which can read out of
> > > array
> > > >
> > > > Fixes:
> > > 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360
> > > >
> > > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  tools/target_dec_fuzzer.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > will apply
> > >
> > 
> > Shouldn't there be warnings about FAST mode if it causes security issues?
> 
> I agree here.
> Either we fix FAST mode, or it must come with important warnings.

I suggest to add a AV_CODEC_FLAG2_FAST_UNSAFE and split the current
uses of the flag up between the 2

will submit a patch doing that unless i hear objections / a better
suggestion.


thx

[...]
Michael Niedermayer May 28, 2020, 12:41 p.m. UTC | #5
On Thu, May 28, 2020 at 02:23:34PM +0200, Michael Niedermayer wrote:
> On Thu, May 28, 2020 at 11:14:15AM +0200, Jean-Baptiste Kempf wrote:
> > On Thu, May 28, 2020, at 00:08, Kieran Kunhya wrote:
> > > On Wed, 27 May 2020 at 22:53, Michael Niedermayer <michael@niedermayer.cc>
> > > wrote:
> > > 
> > > > On Sun, Mar 15, 2020 at 10:20:56PM +0100, Michael Niedermayer wrote:
> > > > > This combination skips allocating large padding which can read out of
> > > > array
> > > > >
> > > > > Fixes:
> > > > 20978/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-5746381832847360
> > > > >
> > > > > Found-by: continuous fuzzing process
> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > > ---
> > > > >  tools/target_dec_fuzzer.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > will apply
> > > >
> > > 
> > > Shouldn't there be warnings about FAST mode if it causes security issues?
> > 
> > I agree here.
> > Either we fix FAST mode, or it must come with important warnings.
> 
> I suggest to add a AV_CODEC_FLAG2_FAST_UNSAFE and split the current
> uses of the flag up between the 2
> 
> will submit a patch doing that unless i hear objections / a better
> suggestion.

ill also disable the use that this would move in master to unsafe,
in release branches

thx

[...]
diff mbox series

Patch

diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index 2487b6ca94..c835293c6a 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -202,7 +202,7 @@  int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
             if (flags & 8)
                 ctx->err_recognition |= AV_EF_EXPLODE;
         }
-        if (flags & 0x10)
+        if ((flags & 0x10) && c->id != AV_CODEC_ID_H264)
             ctx->flags2 |= AV_CODEC_FLAG2_FAST;
 
         if (flags & 0x40)