Re: [FFmpeg-devel] [PATCH 05/11] avcodec/flacdec: Fix signed integre overflow

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 05/11] avcodec/flacdec: Fix signed integre overflow
Date: Sat, 6 May 2023 17:08:13 +0200
Message-ID: <20230506150813.GD4538@pb2> (raw)
In-Reply-To: <cb0a1de4-63bc-b8cc-585a-daa075610d55@gmail.com>

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

On Fri, May 05, 2023 at 07:36:05PM -0300, James Almer wrote:
> On 4/16/2023 1:48 PM, Michael Niedermayer wrote:
> > Fixes: signed integer overflow: 3011809745540902265 + 6323452730883571725 cannot be represented in type 'long'
> > Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FLAC_fuzzer-6687553022722048
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/flacdec.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
> > index cc778a8dff1..524a0469495 100644
> > --- a/libavcodec/flacdec.c
> > +++ b/libavcodec/flacdec.c
> > @@ -513,7 +513,7 @@ static int decode_subframe_lpc_33bps(FLACContext *s, int64_t *decoded,
> >       for (i = pred_order; i < s->blocksize; i++, decoded++) {
> >           int64_t sum = 0;
> >           for (j = 0; j < pred_order; j++)
> > -            sum += (int64_t)coeffs[j] * decoded[j];
> > +            sum += (int64_t)coeffs[j] * (uint64_t)decoded[j];
> 
> Why not instead do
> 
> sum = av_sat_add64(sum, (int64_t)coeffs[j] * decoded[j]);

Why should this be clipping ?
flac is a lossless codec, i see nothing in the specification that calls for
cliping.

> 
> Also, decoded[j] is an int64_t, so wouldn't coeffs[j] be promoted if you
> swap the order in the multiplication, thus saving the cast?

it can be shuffled around to achieve the same, do you prefer 
 coeffs[j] * (uint64_t)decoded[j]; ?

thx

[...] 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr

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