[FFmpeg-devel] [PATCH] aac: buffer overread checks

[Alex Converse]

2010/2/15 M?ns Rullg?rd <mans at mansr.com>:
> Alex Converse <alex.converse at gmail.com> writes:
>
>> Right now the AAC decoder is very prone to segfaulting due to buffer
>> overreads on thrashed streams. The attached patch adds buffer checks
>> after every syntax element and before large jumps forward in the PCE,
>> DSE, and FIL elements. This does not solve all the AAC buffer issues.
>> Zero sized codebook sections remain problematic and we still need more
>> padding possibly with even more invasive checks. Even so this patch
>> prevents the crash associated with the file attached to issue 1295 on
>> my system. (Valgrind still reports overreads but the file no longer
>> segfaults.) In general I think this patch will prevent many crashes
>> due to random stream corruption and mangled frames coming from bad
>> demuxers (I'm looking at you gstreamer.)
>>
>> diff --git a/libavcodec/aac.c b/libavcodec/aac.c
>> index 90581fc..4b13167 100644
>> --- a/libavcodec/aac.c
>> +++ b/libavcodec/aac.c
>> @@ -107,6 +107,8 @@ static VLC vlc_spectral[11];
>>
>> ?static uint32_t cbrt_tab[1<<13];
>>
>> +static const char const *overread_err = "Input buffer exhausted before END element found\n";
>
> Misplaced const, and why not const char foo[]?
>

fixed

> Do any of these checks have a measurable performance impact? ?If not,
> I guess they are OK to apply. ?If there is a performance hit, maybe we
> should look at other approaches.
>

I was unable to find a measurable speed change. These added checks are
on the order of once per syntax element (about the number of channels
each frame).