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

[Paul B Mahol]

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?

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Everything should be made as simple as possible, but not simpler.
> -- Albert Einstein
>