[FFmpeg-devel] [flac] Fix integer-overflow in flac_lpc_33_c

[Michael Niedermayer]

Hi Dale

On Wed, Jul 30, 2025 at 09:36:51AM -0700, Dale Curtis wrote:
> On Wed, Jul 30, 2025 at 3:01 AM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Hi Dale
> >
> > On Tue, Jul 29, 2025 at 03:07:38PM -0700, Dale Curtis wrote:
> > > This fix copies a couple of casts from surrounding functions.
> > > See https://crbug.com/432528781 for stack trace details.
> > >
> > > Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
> >
> > >  flacdsp.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 187b2fdeaecb08d3683b90875f4d7c0e74a38da1  flac_fix_v1.patch
> > > From 0bf245bf8a031d12aec77e68dbc627247255eeb0 Mon Sep 17 00:00:00 2001
> > > From: Dale Curtis <dalecurtis at chromium.org>
> > > Date: Tue, 29 Jul 2025 22:05:19 +0000
> > > Subject: [PATCH] [flac] Fix integer-overflow in flac_lpc_33_c
> > >
> > > This fix copies a couple of casts from surrounding functions.
> >

> > > See https://crbug.com/432528781 for stack trace details.
> >
> > You (email=michael at niedermayer.cc) are not authorized to access this page!
> >
> 
> The bug is public and I can open it in an incognito window, so I'm not sure
> what's going on here. Are you referring to the Clusterfuzz page itself? I
> can add more info to the bug if it's helpful, but can't control ClusterFuzz
> access unfortunately.

you wrote "for stack trace details.", but the stack trace details are on the
Clusterfuzz page

so either the "for stack trace details." should be removed or some stack
trace details  could be added to teh public page


> 
> 
> >
> >
> > [...]
> >
> > > -        decoded[j] = residual[i] + (sum >> qlevel);
> > > +        decoded[j] = (uint64_t)residual[i] + (unsigned)(sum >> qlevel);
> >
> > This does not give the same result for cases that do not overflow
> >
> > I would guess more in the direction of:
> >
> >         decoded[j] = (int64_t)residual[i] + (uint64_t)(sum >> qlevel);
> >
> 
> Happy to make that change, but are one of the following casts also
> incorrect then?

> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/flacdsp.c#L111

Iam not sure the int64_t vs uint64_t affects any audio output, it
does affect a checkasm. So iam not sure about "correct"


> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/flacdsp.c#L69

sum is a int, so -> unsigned should be fine

in the case of the patch sum is a int64_t so casting to unsigned truncates it

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250730/246abffd/attachment.sig>