[FFmpeg-devel] [PATCH] Checked get_bits.h functions to prevent overread

[Laurent Aimar]

Hi,

On Fri, Sep 09, 2011 at 08:16:31AM +0200, Laurent Aimar wrote:
> On Fri, Sep 09, 2011 at 02:26:53AM +0200, Michael Niedermayer wrote:
> > On Fri, Sep 09, 2011 at 02:05:19AM +0200, Laurent Aimar wrote:
> > [...]
> > > > > @@ -311,7 +331,12 @@ static inline unsigned int get_bits1(GetBitContext *s){
> > > > >      result <<= index & 7;
> > > > >      result >>= 8 - 1;
> > > > >  #endif
> > > > > +#ifndef UNCHECK_BITSTREAM_READER
> > > > > +    if (index < s->size_in_bits)
> > > > > +        index++;
> > > > > +#else
> > > > >      index++;
> > > > > +#endif
> > > > 
> > > > i think this will break error detection of some files with some
> > > > decoders because they detect it by an overread having happened
> > > > 
> > > > also it might lead to infinite loops or other unexpected things
> > > > as some decoders depend on them
> > > > hitting zero padding after the buffer which is guranteed to lead to
> > > > vlc decoding failures for them as they have no valid all 0 vlc code
> > >  I see. A simple way could be to simply add 8 * PADDING_SIZE to the check.
> > > I will add that locally.
> > 
> > Iam not sure this is enough
> > the problematic case iam thinking of is a decoder that works with
> > slices, so the guranteed 0 padding would be farther away if the
> > current slice is not the last. mpeg1/2 should be examples of this
> > case
> > 
> > maybe just returning 0 after size+something would work better
>  I wanted to avoid the check while loading the cache, but you're right,
> it's probably the simplest solution to avoid the issue you mentionned.
>  I will provide a patch to do that instead.

 New patch attached and it's a bit simpler.

 Now out of bound index is checked when reading the data and the value 0 is
returned in such cases. get_bits_count() will then return the real number
of bits read.

 I still have an issue with mpegaudio though. I will try to fix it first and
then I will do some benchmarks.

Regards,

-- 
fenrir
-------------- next part --------------
A non-text attachment was scrubbed...
Name: get_bits-p3.patch
Type: text/x-diff
Size: 3342 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110909/c422ac34/attachment.bin>