[FFmpeg-devel] [PATCH] avcodec/get_bits: fix crash with get_bits1()

[Michael Niedermayer]

On Mon, Oct 28, 2013 at 09:37:23PM +0000, Paul B Mahol wrote:
> On 10/28/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Mon, Oct 28, 2013 at 06:20:05PM +0000, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> ---
> >>  libavcodec/get_bits.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> >> index 32715d8..3df570b 100644
> >> --- a/libavcodec/get_bits.h
> >> +++ b/libavcodec/get_bits.h
> >> @@ -410,7 +410,7 @@ static inline int init_get_bits(GetBitContext *s,
> >> const uint8_t *buffer,
> >>
> >>      if (bit_size >= INT_MAX - 7 || bit_size <= 0 || !buffer) {
> >>          buffer_size = bit_size = 0;
> >> -        buffer      = NULL;
> >> +        buffer      = (const uint8_t*)s;
> >>          ret         = AVERROR_INVALIDDATA;
> >>      }
> >
> > that looks a bit strange, s is a pointer to the GetBitContext
> > and by allowing the context and whatever is after it to be read
> > as if it was the source bitstream
> 
> It should not be able to read more than one byte - for checked variant.
> 
> > instead of crashing with a null pointer dereference, an attacker
> > could be able to extract information for example about the memory
> > layout of the running process which could allow him to bypass ASLR
> 
> Do you have anything better to propose?

its not clear to me why you want size = 0 to be treated as
an error. It leads to the rather annoying case that every decoder that
gets a 0 sized input either at packet or chunk level and which passes
this to init_get_bits() suddenly has a new error case to deal with
while now with padding requirements it could saftely read a few
bytes and only needs a bits_left check after that if it reads more



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

There will always be a question for which you do not know the correct answer.
-------------- 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/20131028/9aa6020e/attachment.asc>