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

[Dale Curtis <dalecurtis@chromium.org>]

[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]

Patchset updated with your suggestions. Thanks!

On Wed, Jul 30, 2025 at 12:53 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> 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@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@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@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@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
>

Ah, sorry, I thought ClusterFuzz had included it since it was a low
severity issue. I've updated the bug.


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

Ah, I didn't check the type for sum closely enough. Sorry again!


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

[-- Attachment #2: flac_fix_v2.patch --]
[-- Type: application/x-patch, Size: 1006 bytes --]

[-- Attachment #3: 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".