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

[Michael Niedermayer]

On Sat, Apr 18, 2015 at 10:28:53PM +0200, Andreas Cadhalpun wrote:
> 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?

whatever you prefer, the important part is to add checks where they
are needed

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150418/97f2c34c/attachment.asc>