[FFmpeg-devel] [PATCH] alsdec: validate time diff index

[Andreas Cadhalpun]

On 18.04.2015 21:46, Michael Niedermayer wrote:
> On Sat, Apr 18, 2015 at 09:13:30PM +0200, Andreas Cadhalpun wrote:
>> On 18.04.2015 20:42, Michael Niedermayer wrote:
>>> On Sat, Apr 18, 2015 at 08:13:30PM +0200, Andreas Cadhalpun wrote:
>>>> @@ -1290,8 +1290,16 @@ static int revert_channel_correlation(ALSDecContext *ctx, ALSBlockData *bd,
>>>>  
>>>>              if (ch[dep].time_diff_sign) {
>>>>                  t      = -t;
>>>> +                if (t > 0 && begin < t) {
>>>
>>> time_diff_index is always positive, so t is always negative here
>>
>> I didn't verify this, but I added the 'begin < t' check only for symmetry
>> with the end case.

I verified it now:
    current->time_diff_index = get_bits(gb, ctx->ltp_lag_length - 3) + 3;
...
    ctx->ltp_lag_length = 8 + (avctx->sample_rate >=  96000) +
                              (avctx->sample_rate >= 192000);

So ltp_lag_length is at most 10, i.e. at most 7 bits are read for the
time_diff_index, so it is always positive.

>>> so this cant be true unless the context got corrupted or iam missing
>>> something
>>
>> If you're sure t is always negative here, this check can be dropped.
> 
> maybe add a av_assert0() to protect againt future code changes

I don't think an assert would be good here. If you want to protect against
future code changes, I would rather leave the error return.
But I'd also be fine with dropping this check altogether.
What do you prefer?

Best regards,
Andreas