[FFmpeg-devel] [PATCH] avcodec/webp: Reinitilaize VP8 decoder on pixel format mismatch

[Ronald S. Bultje]

Hi,

On Tue, May 9, 2017 at 9:24 PM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Tue, May 09, 2017 at 09:08:08PM -0400, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Tue, May 9, 2017 at 8:37 PM, Michael Niedermayer
> <michael at niedermayer.cc>
> > wrote:
> >
> > > Fixes: out of array access
> > > Fixes: 1434/clusterfuzz-testcase-minimized-6314998085189632
> > > Fixes: 1435/clusterfuzz-testcase-minimized-6483783723253760
> > >
> > > Found-by: continuous fuzzing process https://github.com/google/oss-
> > > fuzz/tree/master/targets/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavcodec/webp.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> > > index 16c3ae2662..23ed4bc26f 100644
> > > --- a/libavcodec/webp.c
> > > +++ b/libavcodec/webp.c
> > > @@ -1330,12 +1330,17 @@ static int vp8_lossy_decode_frame(
> AVCodecContext
> > > *avctx, AVFrame *p,
> > >      WebPContext *s = avctx->priv_data;
> > >      AVPacket pkt;
> > >      int ret;
> > > +    enum AVPixelFormat wanted_pix_fmt = s->has_alpha ?
> > > AV_PIX_FMT_YUVA420P : AV_PIX_FMT_YUV420P;
> > > +
> > > +    if (s->initialized && wanted_pix_fmt != avctx->pix_fmt) {
> > > +        ff_vp8_decode_free(avctx);
> > > +        s->initialized = 0;
> > > +    }
> > >
> > >      if (!s->initialized) {
> > >          ff_vp8_decode_init(avctx);
> > >          s->initialized = 1;
> > > -        if (s->has_alpha)
> > > -            avctx->pix_fmt = AV_PIX_FMT_YUVA420P;
> > > +        avctx->pix_fmt = wanted_pix_fmt;
> > >      }
> > >      s->lossless = 0;
> >
> >
> > What is the out of array access? webp is intra only and the only thing
> that
> > is initialized with memory in that call is reference frames. What's going
> > on here?
>
> webp uses the same context as VP8, and it changes the pixel format
> as it needs. Vp8 doesnt work if its format is changed under its feet


I think what you're trying to say is that it doesn't work with RGBA as
pixel format (it shouldn't need a reset between yuv420p and yuv420pa). We
indeed don't set pix_fmt if has_alpha = 0, which seems like the core of the
issue. I'm not sure you need to call ff_vp8_decode_init() twice, in fact
I'm pretty sure you don't have to. It may also help to assert that pix_fmt
is yuv420p(a) when calling vp8 functions.

Ronald