[FFmpeg-devel] [PATCH 2/3] avcodec/osq: avoid several signed integer overflows

[James Almer]

On 12/26/2023 1:37 PM, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 178459578 + 2009763270 cannot be represented in type 'int'
> Fixes: 62285/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-5013423686287360
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>   libavcodec/osq.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> index abe15c97f18..f2771c46eb5 100644
> --- a/libavcodec/osq.c
> +++ b/libavcodec/osq.c
> @@ -222,8 +222,8 @@ static int osq_channel_parameters(AVCodecContext *avctx, int ch)
>   #define C (-3)
>   #define D (-4)
>   #define E (-5)
> -#define P2 ((dst[A] + dst[A]) - dst[B])
> -#define P3 ((dst[A] - dst[B]) * 3 + dst[C])
> +#define P2 (((unsigned)dst[A] + dst[A]) - dst[B])
> +#define P3 (((unsigned)dst[A] - dst[B]) * 3 + dst[C])
>   
>   static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int downsample)
>   {
> @@ -273,10 +273,10 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
>               case 0:
>                   break;
>               case 1:
> -                dst[n] += dst[A];
> +                dst[n] += (unsigned)dst[A];
>                   break;
>               case 2:
> -                dst[n] += dst[A] + p;
> +                dst[n] += (unsigned)dst[A] + p;
>                   break;
>               case 3:
>                   dst[n] += P2;
> @@ -291,28 +291,28 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
>                   dst[n] += P3 + p;
>                   break;
>               case 7:
> -                dst[n] += (P2 + P3) / 2 + p;
> +                dst[n] += (int)(P2 + P3) / 2 + (unsigned)p;

Would 2U work for this? It's shorted and more readable that casts 
everywhere. Same for most cases below.

>                   break;
>               case 8:
> -                dst[n] += (P2 + P3) / 2;
> +                dst[n] += (int)(P2 + P3) / 2;
>                   break;
>               case 9:
> -                dst[n] += (P2 * 2 + P3) / 3 + p;
> +                dst[n] += (int)(P2 * 2 + P3) / 3 + (unsigned)p;
>                   break;
>               case 10:
> -                dst[n] += (P2 + P3 * 2) / 3 + p;
> +                dst[n] += (int)(P2 + P3 * 2) / 3 + (unsigned)p;
>                   break;
>               case 11:
> -                dst[n] += (dst[A] + dst[B]) / 2;
> +                dst[n] += (int)((unsigned)dst[A] + dst[B]) / 2;
>                   break;
>               case 12:
> -                dst[n] += dst[B];
> +                dst[n] += (unsigned)dst[B];
>                   break;
>               case 13:
> -                dst[n] += (dst[D] + dst[B]) / 2;
> +                dst[n] += (int)(unsigned)(dst[D] + dst[B]) / 2;
>                   break;
>               case 14:
> -                dst[n] += (P2 + dst[A]) / 2 + p;
> +                dst[n] += (int)((unsigned)P2 + dst[A]) / 2 + (unsigned)p;
>                   break;
>               default:
>                   return AVERROR_INVALIDDATA;