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

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

[Dale Curtis]

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

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>

[-- Attachment #2: flac_fix_v1.patch --]
[-- Type: application/octet-stream, 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".

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

[Michael Niedermayer]

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

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!


[...]

> -        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);


thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell

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

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

[Dale Curtis]

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.


>
>
> [...]
>
> > -        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
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/flacdsp.c#L69


>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> During times of universal deceit, telling the truth becomes a
> revolutionary act. -- George Orwell
> _______________________________________________
> 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".
>
_______________________________________________
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".

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

[Michael Niedermayer]

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

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


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

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

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

[Dale Curtis]

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