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

[Michael Niedermayer]

On Fri, Aug 16, 2019 at 02:57:42PM +0200, Tomas Härdin wrote:
> fre 2019-08-16 klockan 00:50 +0200 skrev 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
> > > > > [...]
> > > > > +    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.
> 
> We should keep in mind the spec is fine with there being zero strips.
> It's only if one wants to support certain decoders that there will/must
> be one or more strips.
> 
> I don't have any way of testing the MacOS decoder, but the old Win3.1
> decoder is easy enough to get running. Then I can also check whether
> strips narrower than avctx->width are OK.
> 
> > > 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 
> 
> I feel like a better fix would be to make the decoder not choke on the
> kind of files that trigger this. With this style of check it's still
> possible to insert a second strip that triggers the slowness, with a
> trivially sized strip before it.
> 

> Would you mind sending me the sample?

will send it privatly

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20190817/4ed29b88/attachment.sig>