[FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

[Tomas Härdin]

ons 2019-08-14 klockan 12:32 +0200 skrev Tomas Härdin:
> mån 2019-08-12 klockan 21:17 +0200 skrev Michael Niedermayer:
> > Fixes: Timeout (12sec -> 32ms)
> > Fixes: 16078/clusterfuzz-testcase-minimized-
> > ffmpeg_AV_CODEC_ID_CINEPAK_fuzzer-5695832885559296
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/cinepak.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
> > index aeb15de0ed..62eb794332 100644
> > --- a/libavcodec/cinepak.c
> > +++ b/libavcodec/cinepak.c
> > @@ -356,6 +356,9 @@ static int cinepak_predecode_check
> > (CinepakContext *s)
> >      if (s->size < 10 + s->sega_film_skip_bytes + num_strips * 12)
> >          return AVERROR_INVALIDDATA;
> >  
> > +    if (s->size < (s->avctx->width * s->avctx->height) / (4*4*8))
> > +        return AVERROR_INVALIDDATA;
> 
> This is wrong if num_strips == 0, and if the MB area is != 0 mod 8. You
> could merge it with the check above into something like:
> 
> if (s->size < 10 + s->sega_film_skip_bytes + num_strips * 12 +
>     (num_strips ? ((s->avctx->width * s->avctx->height) / 16 + 7)/8 :
> 0)) {
>     return AVERROR_INVALIDDATA;
> }
> 
> The check further down could also check each strip's size, not just the
> first one.

Actually, thinking a bit more about this, I suspect it might be legal
to have strips that don't cover the entire frame. It would certainly be
good to support that, for optimizing skip frames. Not sure what old
decoders make of that however. A skip could potentially be encoded in
22 + (width/4 + 7)/8 bytes while still being compatible.

I'd replace s->avctx->height in the expression with the height of the
first strip, to not constrain things too much.

/Tomas