[FFmpeg-devel] [PATCH 2/2] avcodec/zmbv: Check that the raw input is large enough to contain MVs or an intra frame

[Michael Niedermayer]

On Mon, Sep 17, 2018 at 09:58:18AM +0200, Paul B Mahol wrote:
> On 9/17/18, Michael Niedermayer <michael at niedermayer.cc> wrote:
> > On Sun, Sep 16, 2018 at 10:16:05AM +0200, Paul B Mahol wrote:
> >> On 9/16/18, Michael Niedermayer <michael at 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 <michael at 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 ?


> 
> 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
-------------- 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/20180917/c2a14880/attachment.sig>