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

Paul B Mahol onemda at gmail.com
Mon Jun 17 12:54:28 CEST 2013


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. Isn't overflow undefined and thus this check doesn't
make sense?

> %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?

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list