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

[Michael Niedermayer]

On Thu, Aug 15, 2019 at 04:43:19PM +0200, Tomas Härdin wrote:
> 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 was asking myself the same questions before writing the patch, and
in the "spec" there is
"Each frame is segmented into 4x4 pixel blocks, and each block is coded using either 1 or 4 vectors."
"Each" meaning no holes to me. If thats actually true for all real files
that i do not know.


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

ill post a patch doing that 

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190816/1b6e19d8/attachment.sig>