[FFmpeg-devel] [PATCH 2/2] avcodec/vp8: Check for bitsteam end in decode_mb_row_no_filter()

[Michael Niedermayer]

On Tue, Feb 28, 2017 at 07:44:12AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Mon, Feb 27, 2017 at 10:28 PM, Michael Niedermayer <
> michael at niedermayer.cc> wrote:
> 
> > Fixes: 686/clusterfuzz-testcase-5853946876788736
> >
> > 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/vp8.c | 20 ++++++++++++++------
> >  libavcodec/vp8.h |  2 +-
> >  2 files changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> > index c1c3eb7072..cc158528ef 100644
> > --- a/libavcodec/vp8.c
> > +++ b/libavcodec/vp8.c
> > @@ -2275,7 +2275,7 @@ static void vp8_decode_mv_mb_modes(AVCodecContext
> > *avctx, VP8Frame *cur_frame,
> >  #define update_pos(td, mb_y, mb_x) while(0)
> >  #endif
> >
> > -static av_always_inline void decode_mb_row_no_filter(AVCodecContext
> > *avctx, void *tdata,
> > +static av_always_inline int decode_mb_row_no_filter(AVCodecContext
> > *avctx, void *tdata,
> >                                          int jobnr, int threadnr, int
> > is_vp7)
> >  {
> >      VP8Context *s = avctx->priv_data;
> > @@ -2291,6 +2291,10 @@ static av_always_inline void
> > decode_mb_row_no_filter(AVCodecContext *avctx, void
> >          curframe->tf.f->data[1] +  8 * mb_y * s->uvlinesize,
> >          curframe->tf.f->data[2] +  8 * mb_y * s->uvlinesize
> >      };
> > +
> > +    if (c->end <= c->buffer && c->bits >= 0)
> > +         return AVERROR_INVALIDDATA;
> 
> 
> From vp56.h:
> 
>     if(bits >= 0 && c->buffer < c->end) {
>         code_word |= bytestream_get_be16(&c->buffer) << bits;
>         bits -= 16;
>     }
> 
> So this looks supicious, c->end should never be more than 1 byte beyond
> c->buffer (which is padded by AV_INPUT_BUFFER_PADDING_SIZE). What is the
> real issue?

The real issue is that the code runs with blindfolds and hands tied
behind its back, so it doesnt know when its running toward a wall
and in fact it doesnt stop running when it hits the wall not even
once it starved, its even running in its grave.

You can feed the code with approximately a header with huge resolution
and 0 bytes of actual image and it wont notice, and after a really long
time it still wont notice it is decoding a image out of a non existing
bitstream.

That check checks if the end of the bitstream is reached and returns
an error in that case. That way the test sample finished in 10sec
instead of not finishing within the time i waited

If you want i can also add an error message that says that the
bitstream end is reached before the image had finished decoding

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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170228/48caa4a2/attachment.sig>