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

Michael Niedermayer michaelni at gmx.at
Fri Sep 9 02:26:53 CEST 2011


Hi

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

also reading up to 8 * PADDING_SIZE will overread, for example when
25bits are read beginning from the last of 8 * PADDING_SIZE  bits

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20110909/075461cf/attachment.asc>


More information about the ffmpeg-devel mailing list