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

[Michael Niedermayer]

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.
> 
> > 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


> 
> >> +                    av_log(ctx->avctx, AV_LOG_ERROR, "begin %u smaller than time diff index %d.\n", begin, t);
> >> +                    return AVERROR_INVALIDDATA;
> >> +                }
> >>                  begin -= t;
> >>              } else {
> >> +                if (t > 0 && end < t) {
> > 
> > and here t should always be > 0 so the test shouldnt be needed
> 
> I've only seen cases with t > 0, but the t > 0 check makes it explicit that
> end < t is a comparison between unsigned variables. So I would leave it.

sure, ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/46dfdd20/attachment.asc>