[FFmpeg-devel] [PATCH] oggdec: add integer overflow and allocation check in ogg_read_page()

[Michael Niedermayer]

On Mon, May 23, 2011 at 07:15:45PM +0200, Michael Niedermayer wrote:
> On Mon, May 23, 2011 at 06:44:11PM +0200, Stefano Sabatini wrote:
> > On date Monday 2011-05-23 05:15:27 +0200, Michael Niedermayer encoded:
> > > On Mon, May 23, 2011 at 12:04:29AM +0200, Stefano Sabatini wrote:
> > > > ---
> > > >  libavformat/oggdec.c |    8 +++++++-
> > > >  1 files changed, 7 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> > > > index 7f65365..f137b97 100644
> > > > --- a/libavformat/oggdec.c
> > > > +++ b/libavformat/oggdec.c
> > > > @@ -288,7 +288,13 @@ static int ogg_read_page(AVFormatContext *s, int *str)
> > > >      }
> > > >  
> > > >      if (os->bufsize - os->bufpos < size){
> > > > -        uint8_t *nb = av_malloc (os->bufsize *= 2);
> > > > +        uint8_t *nb;
> > > > +        if (os->bufsize > SIZE_MAX/2) {
> > > > +            av_log(s, AV_LOG_ERROR, "Ogg page with size %u is too big\n", os->bufsize);
> > > > +            return AVERROR_INVALIDDATA;
> > > > +        }
> > > > +        if (!(nb = av_malloc(os->bufsize *= 2)))
> > > > +            return AVERROR(ENOMEM);
> > > 
> > > i hope there is a better solution than allocating several gigabyte
> > 
> > Yes, but this at least is fixing a crash.
> 
> please review attached patch
> note this is a RFC, i have not checked if this has sideeffects and
> i do not know why the if() was there.

better fix applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110603/96043bf0/attachment.asc>