[FFmpeg-devel] [PATCH] Fixed size given to init_get_bits() in xan decoder.

[Michael Niedermayer]

On Sat, Sep 10, 2011 at 01:36:56PM +0200, Laurent Aimar wrote:
> On Sat, Sep 10, 2011 at 01:37:44AM +0200, Michael Niedermayer wrote:
> > On Sat, Sep 10, 2011 at 12:40:31AM +0200, Laurent Aimar wrote:
> > > See the title.
> > [...]
> > 
> > > @@ -270,7 +271,8 @@ static void xan_wc3_decode_frame(XanContext *s) {
> > >      vector_segment =    s->buf + AV_RL16(&s->buf[4]);
> > >      imagedata_segment = s->buf + AV_RL16(&s->buf[6]);
> > >  
> > > -    xan_huffman_decode(opcode_buffer, huffman_segment, opcode_buffer_size);
> > > +    xan_huffman_decode(opcode_buffer, opcode_buffer_size,
> > > +                       huffman_segment, s->size - (huffman_segment - s->buf) );
> > 
> > something like the following could be used to find a tighter size bound
> > unsigned hsize= s->size - AV_RL16(s->buf);
> > for(i=2; i<8; i+=2)
> >     hsize= FFMIN(hsize, AV_RL16(s->buf+i) - AV_RL16(s->buf));
> > 
> > also either way it needs something like:
> > for(i=0; i<8; i+=2)
> >     if(AV_RL16(s->buf+i) >= s->size)
> >         return -1;
> > 
> > without that the size could become negative, which doesnt seem right to
> > me
>  Yes I agree that the size should be checked but I don't think it belongs to
> this patch:
>  - with or without this patch, the case where the size becomes < 0 will
>  segfault.
>  - this patch fixed the argument value of init_get_bits

i did not mean to suggest it has to be in the same patch
just that for the ultimate goal of this patch series which is not to
read outside the buffer, it has to be checked too.


> 
> Also, Alex Converse has sent a patch containing the bound checking for the xan
> decoder.

ill merge it once it hits some public git repo somewhere

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

Democracy is the form of government in which you can choose your dictator
-------------- 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/20110910/a5775c48/attachment.asc>