On Wed, Feb 22, 2012 at 10:20 AM, Alex Converse <[email protected]>wrote:

> On Wed, Feb 22, 2012 at 10:01 AM, Alex Converse 
> <[email protected]>wrote:
>
>> On Wed, Feb 22, 2012 at 9:45 AM, Ronald S. Bultje <[email protected]>wrote:
>>
>>> From: "Ronald S. Bultje" <[email protected]>
>>>
>>> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>>> CC: [email protected]
>>> ---
>>>  libavcodec/aacdec.c |   11 ++++++-----
>>>  1 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/aacdec.c b/libavcodec/aacdec.c
>>> index dd9eefc..6cbb637 100644
>>> --- a/libavcodec/aacdec.c
>>> +++ b/libavcodec/aacdec.c
>>> @@ -973,13 +973,14 @@ static int decode_band_types(AACContext *ac, enum
>>> BandType band_type[120],
>>>                 av_log(ac->avctx, AV_LOG_ERROR, "invalid band type\n");
>>>                 return -1;
>>>             }
>>> -            while ((sect_len_incr = get_bits(gb, bits)) == (1 << bits)
>>> - 1)
>>> +            while ((sect_len_incr = get_bits(gb, bits)) == (1 << bits)
>>> - 1) {
>>> +                if (get_bits_left(gb) < 0) {
>>>
>>
> Should we add a (sect_end > ics->max_sfb) check in here? I imagine
> a significantly long string of ones over run could make sect_end overrun.
>
>
>> +                    av_log(ac->avctx, AV_LOG_ERROR, overread_err);
>>> +                    return -1;
>>> +                }
>>>                 sect_end += sect_len_incr;
>>> -            sect_end += sect_len_incr;
>>> -            if (get_bits_left(gb) < 0) {
>>> -                av_log(ac->avctx, AV_LOG_ERROR, overread_err);
>>> -                return -1;
>>>             }
>>> +            sect_end += sect_len_incr;
>>>             if (sect_end > ics->max_sfb) {
>>>                 av_log(ac->avctx, AV_LOG_ERROR,
>>>                        "Number of bands (%d) exceeds limit (%d).\n",
>>> --
>>> 1.7.7.4
>>>
>>>
>> How can the EOF be all high bits? Our 6 bytes of padding are required to
>> be zeroes.
>>
>
> Discussed on IRC... Ok in principle.
>

I think something along the lines of:

            do {
                sect_len_incr = get_bits(gb, bits);
                sect_end += sect_len_incr;
                if (get_bits_left(gb) < 0) {
                    av_log(ac->avctx, AV_LOG_ERROR, overread_err);
                    return -1;
                }
                if (sect_end > ics->max_sfb) {
                    av_log(ac->avctx, AV_LOG_ERROR,
                           "Number of bands (%d) exceeds limit (%d).\n",
                           sect_end, ics->max_sfb);
                    return -1;
                }
            } while (sect_len_incr == (1 << bits) - 1);

makes the most sense. Note the do..while loop will only iterate a handful
of times at most.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to