[FFmpeg-devel] patch for mpegaudiodec.c to prevent buffer read-access overflow

[Michael Niedermayer]

fmOn Thu, Mar 12, 2009 at 12:32:50PM -0500, Francois Oligny-Lemieux wrote:
> On Wed, Mar 11, 2009 at 8:30 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> 
> > On Wed, Mar 11, 2009 at 01:35:31PM -0500, Francois Oligny-Lemieux wrote:
> > > Hi,
> > > I identified a place in mpegaudiodec.c where a crash could (and in my
> > case
> > > was) happening from time to time. The crash will happen when the audio
> > > header is corrupted. The original code was doing buf++ while searching
> > for
> > > the header without any consideration for the buffer end causing an
> > overflow
> > > and eventually a read-access violation. Also after a successful resync,
> > the
> > > code was not adjusting the buffer_size.
> > >
> > > I attached a patch containing the fix I'm using for this problem, but
> > feel
> > > free to make your own changes to it.
> > >
> > > Francois
> >
> > > Index: mpegaudiodec.c
> > > ===================================================================
> > > --- mpegaudiodec.c    (revision 17942)
> > > +++ mpegaudiodec.c    (working copy)
> > > @@ -2264,6 +2264,7 @@
> > >      uint32_t header;
> > >      int out_size;
> > >      OUT_INT *out_samples = data;
> > > +    uint8_t * buf_end = buf + buf_size;
> > >
> > >  retry:
> > >      if(buf_size < HEADER_SIZE)
> > > @@ -2274,8 +2275,12 @@
> > >          buf++;
> > >  //        buf_size--;
> > >          av_log(avctx, AV_LOG_ERROR, "Header missing skipping one
> > byte.\n");
> > > +        if ( buf + 3 > buf_end ){
> > > +            return -1; // will overflow
> > > +        }
> > >          goto retry;
> > >      }
> > > +    buf_size = buf_end - buf;
> >
> > considering that there is a check after retry and you dont fix the existing
> > check but rather add a second messy check
> > rejected
> 
> 
> I see what you mean... then I suggest just restoring the buf_size--. I don't
> know why it was left commented-out in the first place?

ok but this needs a bit of testing unless someone remembers why it was
commented out.

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/967a7ae9/attachment.pgp>