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
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
