Hi,

On Wed, Feb 22, 2012 at 10:59 AM, Alex Converse <[email protected]> wrote:
> 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.

That works also, OK with me.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to