[FFmpeg-devel] [PATCH 2/2] avformat/swfdec: Check outlen before allocation

[Anton Khirnov]

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

-- 
Anton Khirnov