[FFmpeg-devel] [PATCH] avcodec/wavpack: check for overflow

[Michael Niedermayer]

On Mon, Jun 17, 2013 at 10:54:28AM +0000, Paul B Mahol wrote:
> On 6/15/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> > Fix integer overflow in fate
> >
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavcodec/wavpack.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
> > index 47f598a..dd273f7 100644
> > --- a/libavcodec/wavpack.c
> > +++ b/libavcodec/wavpack.c
> > @@ -581,8 +581,14 @@ static inline int wv_unpack_stereo(WavpackFrameContext
> > *s, GetBitContext *gb,
> >                      L2 = L + ((s->decorr[i].weightA * (int64_t)A + 512) >>
> > 10);
> >                      R2 = R + ((s->decorr[i].weightB * (int64_t)B + 512) >>
> > 10);
> >                  } else {
> > -                    L2 = L + ((s->decorr[i].weightA * A + 512) >> 10);
> > -                    R2 = R + ((s->decorr[i].weightB * B + 512) >> 10);
> > +                    int64_t Lt = s->decorr[i].weightA * (int64_t)A + 512;
> > +                    int64_t Rt = s->decorr[i].weightB * (int64_t)B + 512;
> > +                    if ((int32_t)Lt != Lt || (int32_t)Rt != Rt) {
> > +                        av_log(s->avctx, AV_LOG_ERROR, "sample overflow %d
> 
> This looks extremly ugly.

Iam quite aware of that, which is part of the reason why i posted this
(aka do you know a less ugly solution?)


> Isn't overflow undefined and thus this check doesn't
> make sense?

The check detects&prevents the overflow sort of.
If someone knows a better way to detect this, maybe at an earlier stage
it certainly is welcome ...


> 
> > %d\n", A, B);
> > +                        return AVERROR_INVALIDDATA;
> > +                    }
> > +                    L2 = L + (Lt >> 10);
> > +                    R2 = R + (Rt >> 10);
> >                  }
> >                  if (A && L)
> >                      s->decorr[i].weightA -= ((((L ^ A) >> 30) & 2) - 1) *
> > s->decorr[i].delta;
> > --
> > 1.7.9.5
> 
> Also, what about other overflows?

I have no samples that cause overflows elsewhere, or rater iam not
aware of having such sample ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130617/41ea9d7d/attachment.asc>