[FFmpeg-devel] [PATCH 2/6] Check for out of bound reads in xan_huffman_decode() of the xan decoder.

[Laurent Aimar]

On Thu, Sep 29, 2011 at 02:18:18AM +0200, Reimar Döffinger wrote:
> 
> 
> On 29 Sep 2011, at 01:04, fenrir at elivagar.org wrote:
> 
> > From: Laurent Aimar <fenrir at videolan.org>
> > 
> > ---
> > libavcodec/xan.c |    5 ++++-
> > 1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libavcodec/xan.c b/libavcodec/xan.c
> > index 51b4b95..3359102 100644
> > --- a/libavcodec/xan.c
> > +++ b/libavcodec/xan.c
> > @@ -114,7 +114,10 @@ static int xan_huffman_decode(unsigned char *dest, int dest_len,
> >     init_get_bits(&gb, ptr, ptr_len * 8);
> > 
> >     while ( val != 0x16 ) {
> > -        val = src[val - 0x17 + get_bits1(&gb) * byte];
> > +        int idx = val - 0x17 + get_bits1(&gb) * byte;
> > +        if (idx < 0 || idx >= 2 * byte)
> > +            return -1;
> 
> Using unsigned will need one check less.
 Patch attached.

> However I don't know if the condition is actually correct.
I have looked at http://wiki.multimedia.cx/index.php?title=Wing_Commander_III_MVE_Video_Codec
and the section parsed by xan_huffman_decode() is described as follow:

byte 0       number of values in the Huffman tree (should be 22)
bytes 1..44  Huffman tree table
bytes 45..   Huffman-coded data.

So I think it's the right test.

-- 
fenrir
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Check-for-out-of-bound-reads-in-xan_huffman_decode-o.patch
Type: text/x-diff
Size: 892 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110929/4a0fb198/attachment.bin>