[FFmpeg-devel] [PATCH] fix for bfi buffer overread (roundup issue 2497)

[Daniel Kang]

On Thu, Jan 6, 2011 at 4:38 PM, Michael Niedermayer <michaelni at gmx.at>wrote:

>  >  bfi.c |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > f75684d72bc0d5aa9b9e00887eb627ce31b2ac29  bfi_buffer_sanity_check.diff
> > From 21d730f77e910810c1d3b2b69ce167d2da702be9 Mon Sep 17 00:00:00 2001
> > From: Daniel Kang <daniel.d.kang at gmail.com>
> > Date: Wed, 5 Jan 2011 23:46:33 -0500
> > Subject: [PATCH] Sanity check on buffer reads
> >
> > ---
> >  libavcodec/bfi.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavcodec/bfi.c b/libavcodec/bfi.c
> > index 91c8f6d..00631f0 100644
> > --- a/libavcodec/bfi.c
> > +++ b/libavcodec/bfi.c
> > @@ -47,7 +47,7 @@ static av_cold int bfi_decode_init(AVCodecContext *
> avctx)
> >  static int bfi_decode_frame(AVCodecContext * avctx, void *data,
> >                              int *data_size, AVPacket *avpkt)
> >  {
> > -    const uint8_t *buf = avpkt->data;
> > +    const uint8_t *buf = avpkt->data, *buf2 = avpkt->data;
> >      int buf_size = avpkt->size;
> >      BFIContext *bfi = avctx->priv_data;
> >      uint8_t *dst = bfi->dst;
> > @@ -99,6 +99,11 @@ static int bfi_decode_frame(AVCodecContext * avctx,
> void *data,
> >          unsigned int code = byte >> 6;
> >          unsigned int length = byte & ~0xC0;
> >
> > +        if (buf-buf2 >= buf_size) {
> > +            av_log(NULL, AV_LOG_ERROR, "Input resolution larger than
> actual frame.\n");
>                      ^^^^
> should be avctx, so the user knows from where the error message comes from.
>
> also with buf_end= avpkt->data + buf_size
> the check becomes buf >= buf_end which is a bit simpler and maybe slightly
> more
> readable though thats nitpicking
>
> also the following can still overread:
>        switch (code) {
>
>        case 0:                //Normal Chain
>            bytestream_get_buffer(&buf, dst, length);
>            dst += length;
>

I have updated the patch to use buf_end and include a second check.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bfi_buffer_sanity_check.diff
Type: application/octet-stream
Size: 1763 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110106/733adfc9/attachment.obj>