On 9/17/18, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Mon, Sep 17, 2018 at 09:58:18AM +0200, Paul B Mahol wrote: >> On 9/17/18, Michael Niedermayer <mich...@niedermayer.cc> wrote: >> > On Sun, Sep 16, 2018 at 10:16:05AM +0200, Paul B Mahol wrote: >> >> On 9/16/18, Michael Niedermayer <mich...@niedermayer.cc> wrote: >> >> > Fixes: Timeout >> >> > Fixes: >> >> > 10182/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ZMBV_fuzzer-6245951174344704 >> >> > >> >> > Found-by: continuous fuzzing process >> >> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> >> > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >> >> > --- >> >> > libavcodec/zmbv.c | 11 ++++++++++- >> >> > 1 file changed, 10 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/libavcodec/zmbv.c b/libavcodec/zmbv.c >> >> > index 9e27a2caad..1133bdf7ba 100644 >> >> > --- a/libavcodec/zmbv.c >> >> > +++ b/libavcodec/zmbv.c >> >> > @@ -409,6 +409,7 @@ static int decode_frame(AVCodecContext *avctx, >> >> > void >> >> > *data, int *got_frame, AVPac >> >> > int zret = Z_OK; // Zlib return code >> >> > int len = buf_size; >> >> > int hi_ver, lo_ver, ret; >> >> > + int min_size; >> >> > >> >> > /* parse header */ >> >> > if (len < 1) >> >> > @@ -510,7 +511,11 @@ static int decode_frame(AVCodecContext *avctx, >> >> > void >> >> > *data, int *got_frame, AVPac >> >> > memset(c->prev, 0, avctx->width * avctx->height * (c->bpp / >> >> > 8)); >> >> > c->decode_intra= decode_intra; >> >> > } >> >> > - >> >> > + if (c->flags & ZMBV_KEYFRAME) { >> >> > + min_size = avctx->width * avctx->height * (c->bpp / 8); >> >> >> >> This is pure guessing? >> > >> > theres a bit of logic behind it but ultimatly yes, its guessing >> > >> > The logic >> > If the input is smaller for a raw keyframe the buffer would not be >> > fully updated. >> > If the buffer is not fully updated then this should be a inter frame >> > not a keyframe. >> > >> > The 2nd part, is that all files i found >> > (http://samples.mplayerhq.hu/V-codecs/ZMBV/) >> > follow that assumtation >> > >> > The 3rd is that the source code for zmbv i could fine (dosbox) >> > seems to never write uncompressed keyframes. >> > >> > The 4th is that the multimedia wiki doesnt seem to say anything about >> > the >> > uncompressed keyframe size >> > >> > so i thought its better to use this as the minimal size and avoid the >> > issue the fuzzer found. >> > >> > Its quite possible that i have missed something. >> > Also we could ask for a sample in the failure case or do something else >> > if >> > you >> > have some other suggestion ? >> > >> Yes, drop patch and leave code as is. > > I would prefer to fix the issue in some form. Do you have some suggestion > or preferrance about how to fix this issue ?
No, you are not interested in fixing real issues. Please do not commit hacks like this patch. Nothing in zlib guarantees that size will fit under your random constraints. > > >> >> Also why this codec needs fuzzing? > > it needs fuzzing as much or as little as any other codec. Theres > nothing special on it > > Also as i was looking over this earlier today, i could improve the check > by also including compressed frames. And i found a kind of unrelated > bug in the codec. Ill resend the related parts of the patchset > > Thanks > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The smallest minority on earth is the individual. Those who deny > individual rights cannot claim to be defenders of minorities. - Ayn Rand > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
