| Message ID | 20210122140947.18479-2-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 1957095e808b80cbe607347a6f23207d6318a48d |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/mxfdec: Fix integer overflow in next position in mxf_read_local_tags() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | warning | Make fate failed |
Quoting Michael Niedermayer (2021-01-22 15:09:47) > Fixes: Timeout (too long -> 241ms) > Fixes: 29083/clusterfuzz-testcase-minimized-ffmpeg_dem_SWF_fuzzer-6273684478230528 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/swfdec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c > index 1463f0ad4d..aa4be88f91 100644 > --- a/libavformat/swfdec.c > +++ b/libavformat/swfdec.c > @@ -367,6 +367,9 @@ static int swf_read_packet(AVFormatContext *s, AVPacket *pkt) > ff_dlog(s, "bitmap: ch=%d fmt=%d %dx%d (linesize=%d) len=%d->%ld pal=%d\n", > ch_id, bmp_fmt, width, height, linesize, len, out_len, colormapsize); > > + if (len * 17373LL < out_len) Where does the magic number come from?
On Sat, Jan 23, 2021 at 03:29:38PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-01-22 15:09:47) > > Fixes: Timeout (too long -> 241ms) > > Fixes: 29083/clusterfuzz-testcase-minimized-ffmpeg_dem_SWF_fuzzer-6273684478230528 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/swfdec.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c > > index 1463f0ad4d..aa4be88f91 100644 > > --- a/libavformat/swfdec.c > > +++ b/libavformat/swfdec.c > > @@ -367,6 +367,9 @@ static int swf_read_packet(AVFormatContext *s, AVPacket *pkt) > > ff_dlog(s, "bitmap: ch=%d fmt=%d %dx%d (linesize=%d) len=%d->%ld pal=%d\n", > > ch_id, bmp_fmt, width, height, linesize, len, out_len, colormapsize); > > > > + if (len * 17373LL < out_len) > > Where does the magic number come from? A very quick simulation of the best case compression for "compress" below is not nice written code as i did not expect I or anyone else would ever see it again I would have preferred some nicer expression or course, but thats what it seems to be asymptotically. For smaller amounts of data a tighter bound is possible but i saw no nice way to consider that and it seems also overkill to try to do it more fine grained for just this main(){ int64_t bits = 0; int bank = 256; int bitbank = 8; for(unsigned i = 0; i<1024*1024*1024*4U-100000;) { int word_size = bank-255; i += word_size; bits += bitbank; if (!(bank & (bank-1))) bitbank ++; bank++; if (bitbank > 16) { printf("BEST %f \n", 8.0 * i / bits ); bank = 256; bitbank = 8; } } } above assumes i remembered correctly how the algorithm works but the value was close to what actual compession of zeros gave [...]
Quoting Michael Niedermayer (2021-01-23 23:34:19) > On Sat, Jan 23, 2021 at 03:29:38PM +0100, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2021-01-22 15:09:47) > > > Fixes: Timeout (too long -> 241ms) > > > Fixes: 29083/clusterfuzz-testcase-minimized-ffmpeg_dem_SWF_fuzzer-6273684478230528 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/swfdec.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c > > > index 1463f0ad4d..aa4be88f91 100644 > > > --- a/libavformat/swfdec.c > > > +++ b/libavformat/swfdec.c > > > @@ -367,6 +367,9 @@ static int swf_read_packet(AVFormatContext *s, AVPacket *pkt) > > > ff_dlog(s, "bitmap: ch=%d fmt=%d %dx%d (linesize=%d) len=%d->%ld pal=%d\n", > > > ch_id, bmp_fmt, width, height, linesize, len, out_len, colormapsize); > > > > > > + if (len * 17373LL < out_len) > > > > Where does the magic number come from? > > A very quick simulation of the best case compression for "compress" > below is not nice written code as i did not expect I or anyone else > would ever see it again > > I would have preferred some nicer expression or course, but thats > what it seems to be asymptotically. For smaller amounts of data a > tighter bound is possible but i saw no nice way to consider that > and it seems also overkill to try to do it more fine grained for > just this > > main(){ > int64_t bits = 0; > int bank = 256; > int bitbank = 8; > for(unsigned i = 0; i<1024*1024*1024*4U-100000;) { > int word_size = bank-255; > i += word_size; > bits += bitbank; > > if (!(bank & (bank-1))) > bitbank ++; > bank++; > if (bitbank > 16) { > printf("BEST %f \n", 8.0 * i / bits ); > bank = 256; > bitbank = 8; > } > } > } > > above assumes i remembered correctly how the algorithm works but the > value was close to what actual compession of zeros gave People who read this code in the future will be interested in all this. So the content of your reply should be added to the commit message and/or the code itself.
On Sun, Jan 24, 2021 at 02:05:17PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-01-23 23:34:19) > > On Sat, Jan 23, 2021 at 03:29:38PM +0100, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2021-01-22 15:09:47) > > > > Fixes: Timeout (too long -> 241ms) > > > > Fixes: 29083/clusterfuzz-testcase-minimized-ffmpeg_dem_SWF_fuzzer-6273684478230528 > > > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavformat/swfdec.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c > > > > index 1463f0ad4d..aa4be88f91 100644 > > > > --- a/libavformat/swfdec.c > > > > +++ b/libavformat/swfdec.c > > > > @@ -367,6 +367,9 @@ static int swf_read_packet(AVFormatContext *s, AVPacket *pkt) > > > > ff_dlog(s, "bitmap: ch=%d fmt=%d %dx%d (linesize=%d) len=%d->%ld pal=%d\n", > > > > ch_id, bmp_fmt, width, height, linesize, len, out_len, colormapsize); > > > > > > > > + if (len * 17373LL < out_len) > > > > > > Where does the magic number come from? > > > > A very quick simulation of the best case compression for "compress" > > below is not nice written code as i did not expect I or anyone else > > would ever see it again > > > > I would have preferred some nicer expression or course, but thats > > what it seems to be asymptotically. For smaller amounts of data a > > tighter bound is possible but i saw no nice way to consider that > > and it seems also overkill to try to do it more fine grained for > > just this > > > > main(){ > > int64_t bits = 0; > > int bank = 256; > > int bitbank = 8; > > for(unsigned i = 0; i<1024*1024*1024*4U-100000;) { > > int word_size = bank-255; > > i += word_size; > > bits += bitbank; > > > > if (!(bank & (bank-1))) > > bitbank ++; > > bank++; > > if (bitbank > 16) { > > printf("BEST %f \n", 8.0 * i / bits ); > > bank = 256; > > bitbank = 8; > > } > > } > > } > > > > above assumes i remembered correctly how the algorithm works but the > > value was close to what actual compession of zeros gave > > People who read this code in the future will be interested in all this. > So the content of your reply should be added to the commit message > and/or the code itself. will apply with the message in the commit message thx [...]
diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c index 1463f0ad4d..aa4be88f91 100644 --- a/libavformat/swfdec.c +++ b/libavformat/swfdec.c @@ -367,6 +367,9 @@ static int swf_read_packet(AVFormatContext *s, AVPacket *pkt) ff_dlog(s, "bitmap: ch=%d fmt=%d %dx%d (linesize=%d) len=%d->%ld pal=%d\n", ch_id, bmp_fmt, width, height, linesize, len, out_len, colormapsize); + if (len * 17373LL < out_len) + goto bitmap_end_skip; + zbuf = av_malloc(len); if (!zbuf) { res = AVERROR(ENOMEM);
Fixes: Timeout (too long -> 241ms) Fixes: 29083/clusterfuzz-testcase-minimized-ffmpeg_dem_SWF_fuzzer-6273684478230528 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/swfdec.c | 3 +++ 1 file changed, 3 insertions(+)