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

[Alex Converse]

On Mon, Feb 15, 2010 at 11:00 AM, Alex Converse <alex.converse at gmail.com> wrote:
>
> 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).

Applied