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

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/3] avcodec/osq: avoid several signed integer overflows
Date: Thu, 28 Dec 2023 20:53:14 +0100
Message-ID: <20231228195314.GX6420@pb2> (raw)
In-Reply-To: <e6710561-1222-49c6-bb9f-5dc91d2187e1@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2625 bytes --]

On Tue, Dec 26, 2023 at 01:56:31PM -0300, James Almer wrote:
> 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@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.

unsigned and signed division are different
-1 / 2 == 0
-1 / 2U != 0

thx

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".