[FFmpeg-devel] [libav-devel] [PATCH 5/6] Fixed segfaults on corruped smaker streams in the decoder.

[Michael Niedermayer]

On Tue, Sep 13, 2011 at 12:04:22AM +0200, Laurent Aimar wrote:
> On Mon, Sep 12, 2011 at 11:59:56PM +0200, Michael Niedermayer wrote:
> > On Mon, Sep 12, 2011 at 11:43:18PM +0200, Laurent Aimar wrote:
> > > On Mon, Sep 12, 2011 at 11:28:44PM +0200, Reimar Döffinger wrote:
> > > > On Sun, Sep 11, 2011 at 07:56:46PM +0200, Laurent Aimar wrote:
> > > > > @@ -653,6 +659,8 @@ static int smka_decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> > > > >      } else { //8-bit data
> > > > >          for(i = stereo; i >= 0; i--)
> > > > >              pred[i] = get_bits(&gb, 8);
> > > > > +        if (stereo + unp_size > data_size)
> > > > > +            return -1;
> > > > 
> > > > This can overflow.
> > > > if (unp_size < 0 || unp_size > data_size - stereo)
> > > > should probably be safe.
> > >  No it doesn't because of the surrounding code BUT I saw a bug
> > > in this patch (data_size is a pointer, a '*' is missing)
> > 
> > fixed
> > 
> > 
> > > 
> > >  It can also be made a bit simpler. I will propose a better patch
> > > later.
> > 
> > i guess you plan something like: ?
> Nearly, this condition is a tad too strict when bits is 1.
> I think it should be (unp_size & ~bits) + stereo > *data_size

i think it should be:
(unp_size & ~bits) + stereo*(bits+1) > *data_size


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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/20110913/958f4b61/attachment.asc>