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

[Laurent Aimar]

On Tue, Sep 13, 2011 at 12:16:50AM +0200, Michael Niedermayer wrote:
> 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

... yes :)

-- 
fenrir