| Message ID | 20210123221056.3366-4-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avutil/common: Add FFABSU() for a signed -> unsigned ABS | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Quoting Michael Niedermayer (2021-01-23 23:10:54) > Fixes: out of array access > Fixes: 29202/clusterfuzz-testcase-minimized-ffmpeg_dem_KUX_fuzzer-5112845840809984 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/flvdec.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > index 07ef342278..e15be0a221 100644 > --- a/libavformat/flvdec.c > +++ b/libavformat/flvdec.c > @@ -41,6 +41,8 @@ > > #define RESYNC_BUFFER_SIZE (1<<20) > > +#define MAX_DEPTH 10 Why 10 specifically. And which buffer overflows?
On Sun, Jan 24, 2021 at 02:14:43PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-01-23 23:10:54) > > Fixes: out of array access > > Fixes: 29202/clusterfuzz-testcase-minimized-ffmpeg_dem_KUX_fuzzer-5112845840809984 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/flvdec.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > > index 07ef342278..e15be0a221 100644 > > --- a/libavformat/flvdec.c > > +++ b/libavformat/flvdec.c > > @@ -41,6 +41,8 @@ > > > > #define RESYNC_BUFFER_SIZE (1<<20) > > > > +#define MAX_DEPTH 10 > > Why 10 specifically. 10 is arbitrary, we could pick 5 or 100 probably > And which buffer overflows? stack [...]
Quoting Michael Niedermayer (2021-01-25 21:29:21) > On Sun, Jan 24, 2021 at 02:14:43PM +0100, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2021-01-23 23:10:54) > > > Fixes: out of array access > > > Fixes: 29202/clusterfuzz-testcase-minimized-ffmpeg_dem_KUX_fuzzer-5112845840809984 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/flvdec.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > > > index 07ef342278..e15be0a221 100644 > > > --- a/libavformat/flvdec.c > > > +++ b/libavformat/flvdec.c > > > @@ -41,6 +41,8 @@ > > > > > > #define RESYNC_BUFFER_SIZE (1<<20) > > > > > > +#define MAX_DEPTH 10 > > > > Why 10 specifically. > > 10 is arbitrary, we could pick 5 or 100 probably or a round number like 16 :) > > > And which buffer overflows? > > stack Okay, please add a comment like // arbitrary limit to prevent unbounded recursion
On Tue, Jan 26, 2021 at 04:37:38PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-01-25 21:29:21) > > On Sun, Jan 24, 2021 at 02:14:43PM +0100, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2021-01-23 23:10:54) > > > > Fixes: out of array access > > > > Fixes: 29202/clusterfuzz-testcase-minimized-ffmpeg_dem_KUX_fuzzer-5112845840809984 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavformat/flvdec.c | 5 +++++ > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > > > > index 07ef342278..e15be0a221 100644 > > > > --- a/libavformat/flvdec.c > > > > +++ b/libavformat/flvdec.c > > > > @@ -41,6 +41,8 @@ > > > > > > > > #define RESYNC_BUFFER_SIZE (1<<20) > > > > > > > > +#define MAX_DEPTH 10 > > > > > > Why 10 specifically. > > > > 10 is arbitrary, we could pick 5 or 100 probably > > or a round number like 16 :) > > > > > > And which buffer overflows? > > > > stack > > Okay, please add a comment like > // arbitrary limit to prevent unbounded recursion will apply with that thx [...]
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c index 07ef342278..e15be0a221 100644 --- a/libavformat/flvdec.c +++ b/libavformat/flvdec.c @@ -41,6 +41,8 @@ #define RESYNC_BUFFER_SIZE (1<<20) +#define MAX_DEPTH 10 + typedef struct FLVContext { const AVClass *class; ///< Class for private options. int trust_metadata; ///< configure streams according onMetaData @@ -493,6 +495,9 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream, double num_val; amf_date date; + if (depth > MAX_DEPTH) + return AVERROR_PATCHWELCOME; + num_val = 0; ioc = s->pb; if (avio_feof(ioc))
Fixes: out of array access Fixes: 29202/clusterfuzz-testcase-minimized-ffmpeg_dem_KUX_fuzzer-5112845840809984 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/flvdec.c | 5 +++++ 1 file changed, 5 insertions(+)