[FFmpeg-devel] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[FFmpeg-devel] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Michael Niedermayer]

ff_aac_usac_config_decode() needs AACDecContext to be set but some callers
pass NULL.

I have no real testcase to implement/test this, so failing in this case
seems safest.

Fixes: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
Fixes: 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/aac/aacdec_usac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
index 132ffee9c20..4856c1786b7 100644
--- a/libavcodec/aac/aacdec_usac.c
+++ b/libavcodec/aac/aacdec_usac.c
@@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac, AVCodecContext *avctx,
     int map_pos_set = 0;
     uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
 
+    if (!ac)
+        return AVERROR_PATCHWELCOME;
+
     memset(usac, 0, sizeof(*usac));
 
     freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
-- 
2.45.2

_______________________________________________
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] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow changes to parameter sets between slices

[Michael Niedermayer]

The slice code detects changes by comparing the pps index.
That way the code cannot detect changes if the sets can change.

Fixes: out of array access
Fixes: 69585/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6389604524556288
Fixes: 69601/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5069068613255168
Fixes: 69621/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6187334182174720

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hevc/hevcdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 1d2e53afc32..d68d454537a 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -3221,17 +3221,20 @@ static int decode_nal_unit(HEVCContext *s, const H2645NAL *nal)
         ret = ff_hevc_decode_nal_vps(&gb, s->avctx, &s->ps);
         if (ret < 0)
             goto fail;
+        s->sh.pps_id = -1;
         break;
     case HEVC_NAL_SPS:
         ret = ff_hevc_decode_nal_sps(&gb, s->avctx, &s->ps,
                                      s->apply_defdispwin);
         if (ret < 0)
             goto fail;
+        s->sh.pps_id = -1;
         break;
     case HEVC_NAL_PPS:
         ret = ff_hevc_decode_nal_pps(&gb, s->avctx, &s->ps);
         if (ret < 0)
             goto fail;
+        s->sh.pps_id = -1;
         break;
     case HEVC_NAL_SEI_PREFIX:
     case HEVC_NAL_SEI_SUFFIX:
-- 
2.45.2

_______________________________________________
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] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow changes to parameter sets between slices

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-24 01:01:34)
> The slice code detects changes by comparing the pps index.
> That way the code cannot detect changes if the sets can change.
> 
> Fixes: out of array access
> Fixes: 69585/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6389604524556288
> Fixes: 69601/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5069068613255168
> Fixes: 69621/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6187334182174720
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/hevc/hevcdec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index 1d2e53afc32..d68d454537a 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -3221,17 +3221,20 @@ static int decode_nal_unit(HEVCContext *s, const H2645NAL *nal)
>          ret = ff_hevc_decode_nal_vps(&gb, s->avctx, &s->ps);
>          if (ret < 0)
>              goto fail;
> +        s->sh.pps_id = -1;

This is backwards. If the problem is that the current check does not
detect the change, then the check should be fixed.

-- 
Anton Khirnov
_______________________________________________
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] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow changes to parameter sets between slices

[Michael Niedermayer]

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

On Tue, Jun 25, 2024 at 10:59:14AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-06-24 01:01:34)
> > The slice code detects changes by comparing the pps index.
> > That way the code cannot detect changes if the sets can change.
> > 
> > Fixes: out of array access
> > Fixes: 69585/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6389604524556288
> > Fixes: 69601/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5069068613255168
> > Fixes: 69621/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6187334182174720
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/hevc/hevcdec.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> > index 1d2e53afc32..d68d454537a 100644
> > --- a/libavcodec/hevc/hevcdec.c
> > +++ b/libavcodec/hevc/hevcdec.c
> > @@ -3221,17 +3221,20 @@ static int decode_nal_unit(HEVCContext *s, const H2645NAL *nal)
> >          ret = ff_hevc_decode_nal_vps(&gb, s->avctx, &s->ps);
> >          if (ret < 0)
> >              goto fail;
> > +        s->sh.pps_id = -1;
> 
> This is backwards. If the problem is that the current check does not
> detect the change, then the check should be fixed.

The specification explcitly doesnt allow the active *PS to change

"Any PPS NAL unit containing the value of pps_pic_parameter_set_id for the active PPS RBSP for a coded picture (and
consequently for the layer containing the coded picture) shall have the same content as that of the active PPS RBSP for the
coded picture, unless it follows the last VCL NAL unit of the coded picture and precedes the first VCL NAL unit of another
coded picture."

"Any SPS NAL unit with nuh_layer_id equal to 0 containing the value of sps_seq_parameter_set_id for the active SPS
RBSP for the base layer for a CVS shall have the same content as that of the active SPS RBSP for the base layer for the
CVS, unless it follows the last access unit of the CVS and precedes the first VCL NAL unit and the first SEI NAL unit
containing an active parameter sets SEI message (when present) of another CVS."

"Any SPS NAL unit with any nuh_layer_id value containing the value of sps_seq_parameter_set_id for the active SPS
RBSP for a particular layer shall have the same content as that of the active SPS RBSP for the particular layer unless it
follows the access unit auA containing the last coded picture for which the active SPS RBSP for the particular layer is
required to be active for the particular layer and precedes the first NAL unit succeeding auA in decoding order that activates
an SPS RBSP with the same value of seq_parameter_set_id."

So if anything fancy is wanted, the way to go is not to parse it in the first place.
Because if it matches then the parsing was a waste of time. If it mismatches, that
would be invalid and would just lead to failure anyway

But really the primary goal is to fix the out of array access and be easy to
backport. I am not trying to fix this in a fancy way.
The simpler and more robust the fix is the better.

thx

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

[-- 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] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow changes to parameter sets between slices

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-26 01:27:36)
> The specification explcitly doesnt allow the active *PS to change
> 
> "Any PPS NAL unit containing the value of pps_pic_parameter_set_id for the active PPS RBSP for a coded picture (and
> consequently for the layer containing the coded picture) shall have the same content as that of the active PPS RBSP for the
> coded picture, unless it follows the last VCL NAL unit of the coded picture and precedes the first VCL NAL unit of another
> coded picture."
> 
> "Any SPS NAL unit with nuh_layer_id equal to 0 containing the value of sps_seq_parameter_set_id for the active SPS
> RBSP for the base layer for a CVS shall have the same content as that of the active SPS RBSP for the base layer for the
> CVS, unless it follows the last access unit of the CVS and precedes the first VCL NAL unit and the first SEI NAL unit
> containing an active parameter sets SEI message (when present) of another CVS."
> 
> "Any SPS NAL unit with any nuh_layer_id value containing the value of sps_seq_parameter_set_id for the active SPS
> RBSP for a particular layer shall have the same content as that of the active SPS RBSP for the particular layer unless it
> follows the access unit auA containing the last coded picture for which the active SPS RBSP for the particular layer is
> required to be active for the particular layer and precedes the first NAL unit succeeding auA in decoding order that activates
> an SPS RBSP with the same value of seq_parameter_set_id."

I have no idea why you're quoting the spec at me, I know perfectly well
the PPS cannot change between slices. My point is this: if the current
check for PPS change does not detect it, it should be fixed.
Specifically, it should compare actual PPS objects rather than pps_id.

> So if anything fancy is wanted, the way to go is not to parse it in the first place.
> Because if it matches then the parsing was a waste of time. If it mismatches, that
> would be invalid and would just lead to failure anyway
> 
> But really the primary goal is to fix the out of array access and be easy to
> backport. I am not trying to fix this in a fancy way.
> The simpler and more robust the fix is the better.

This "simple and robust" approach of sprinkling random checks all over
the place leads to unreadable and unmaintainable code.

-- 
Anton Khirnov
_______________________________________________
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] [PATCH 3/5] avcodec/hevc/hevcdec: SPS not set (or cleared) after frame start

[Michael Niedermayer]

Fixes: NULL pointer dereference
Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hevc/hevcdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index d68d454537a..4dd84d4953c 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -3180,6 +3180,9 @@ static int decode_slice(HEVCContext *s, const H2645NAL *nal, GetBitContext *gb)
     } else if (!s->cur_frame) {
         av_log(s->avctx, AV_LOG_ERROR, "First slice in a frame missing.\n");
         return AVERROR_INVALIDDATA;
+    } else if (!s->ps.sps) {
+        av_log(s->avctx, AV_LOG_ERROR, "sps not set in slice.\n");
+        return AVERROR_INVALIDDATA;
     }
 
     if (s->nal_unit_type != s->first_nal_type) {
-- 
2.45.2

_______________________________________________
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] [PATCH 3/5] avcodec/hevc/hevcdec: SPS not set (or cleared) after frame start

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-24 01:01:35)
> Fixes: NULL pointer dereference
> Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024

seems wrong

-- 
Anton Khirnov
_______________________________________________
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] [PATCH 3/5] avcodec/hevc/hevcdec: SPS not set (or cleared) after frame start

[Michael Niedermayer]

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

On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-06-24 01:01:35)
> > Fixes: NULL pointer dereference
> > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024
> 
> seems wrong

Quite possible, but also your comment seems designed to be unhelpfull
you leave the reader guessing what issue you saw exactly and why and
what you think is better.

Not saying that i couldnt guess but it simply would be nice if you
wouldnt omit such details

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus

[-- 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] [PATCH 3/5] avcodec/hevc/hevcdec: SPS not set (or cleared) after frame start

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-26 01:52:30)
> On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2024-06-24 01:01:35)
> > > Fixes: NULL pointer dereference
> > > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024
> > 
> > seems wrong
> 
> Quite possible, but also your comment seems designed to be unhelpfull
> you leave the reader guessing what issue you saw exactly and why and
> what you think is better.

My comment is designed to be as helpful as your commit message.

"Fixes: NULL pointer dereference" says almost nothing about what
actually goes wrong. It should be impossible to get to that point with
the SPS being unset. Assuming it somehow does happen, the correct fix is
to prevent it from happening, not add random checks to random places.

-- 
Anton Khirnov
_______________________________________________
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] [PATCH 3/5] avcodec/hevc/hevcdec: SPS not set (or cleared) after frame start

[Michael Niedermayer]

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

On Wed, Jun 26, 2024 at 08:38:43AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-06-26 01:52:30)
> > On Tue, Jun 25, 2024 at 11:00:44AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2024-06-24 01:01:35)
> > > > Fixes: NULL pointer dereference
> > > > Fixes: 69623/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-6549698459009024
> > > 
> > > seems wrong
> > 
> > Quite possible, but also your comment seems designed to be unhelpfull
> > you leave the reader guessing what issue you saw exactly and why and
> > what you think is better.
> 
> My comment is designed to be as helpful as your commit message.

If you find my commit message inadequate, you should state that.
Simply ommiting information you know in a reply is not efficient
in moving a review forward


> 
> "Fixes: NULL pointer dereference" says almost nothing about what
> actually goes wrong. It should be impossible to get to that point with
> the SPS being unset. Assuming it somehow does happen, the correct fix is
> to prevent it from happening, not add random checks to random places.

If it still reproduces when i look next time and noone else fixed it
before then ill investigate what exactly is going on.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates

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

[FFmpeg-devel] [PATCH 4/5] avcodec/hevc/hevcdec: Do not allow slices to depend on failed slices

[Michael Niedermayer]

An alternative would be to leave the context unchanged on failure of hls_slice_header()

Fixes: out of array access
Fixes: NULL pointer dereference
Fixes: 69584/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5931086299856896
Fixes: 69724/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5104066422702080

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hevc/hevcdec.c | 5 +++++
 libavcodec/hevc/hevcdec.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 4dd84d4953c..0e0a910641f 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -626,6 +626,10 @@ static int hls_slice_header(SliceHeader *sh, const HEVCContext *s, GetBitContext
 
         if (pps->dependent_slice_segments_enabled_flag)
             sh->dependent_slice_segment_flag = get_bits1(gb);
+        if (sh->dependent_slice_segment_flag && sh->slice_failure) {
+            av_log(s->avctx, AV_LOG_ERROR, "dependent slice failed\n");
+            return AVERROR_INVALIDDATA;
+        }
 
         slice_address_length = av_ceil_log2(sps->ctb_width *
                                             sps->ctb_height);
@@ -3157,6 +3161,7 @@ static int decode_slice(HEVCContext *s, const H2645NAL *nal, GetBitContext *gb)
     int ret;
 
     ret = hls_slice_header(&s->sh, s, gb);
+    s->sh.slice_failure = ret;
     if (ret < 0)
         return ret;
 
diff --git a/libavcodec/hevc/hevcdec.h b/libavcodec/hevc/hevcdec.h
index da4d83e6615..91f20ea26ab 100644
--- a/libavcodec/hevc/hevcdec.h
+++ b/libavcodec/hevc/hevcdec.h
@@ -213,6 +213,7 @@ typedef struct SliceHeader {
 
     uint8_t first_slice_in_pic_flag;
     uint8_t dependent_slice_segment_flag;
+    uint8_t slice_failure;
     uint8_t pic_output_flag;
     uint8_t colour_plane_id;
 
-- 
2.45.2

_______________________________________________
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] [PATCH 4/5] avcodec/hevc/hevcdec: Do not allow slices to depend on failed slices

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-24 01:01:36)
> An alternative would be to leave the context unchanged on failure of hls_slice_header()

There's already slice_initialized.

-- 
Anton Khirnov
_______________________________________________
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] [PATCH 5/5] avformat/iamf: Check substreams in ff_iamf_free_audio_element()

[Michael Niedermayer]

Fixes: member access within null pointer of type 'IAMFSubStream' (aka 'struct IAMFSubStream')
Fixes: 69795/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6216287009701888

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/iamf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/iamf.c b/libavformat/iamf.c
index 5de70dc0828..364cb60e021 100644
--- a/libavformat/iamf.c
+++ b/libavformat/iamf.c
@@ -74,8 +74,10 @@ void ff_iamf_free_audio_element(IAMFAudioElement **paudio_element)
     if (!audio_element)
         return;
 
-    for (int i = 0; i < audio_element->nb_substreams; i++)
-        avcodec_parameters_free(&audio_element->substreams[i].codecpar);
+    if (audio_element->substreams)
+        for (int i = 0; i < audio_element->nb_substreams; i++) {
+            avcodec_parameters_free(&audio_element->substreams[i].codecpar);
+        }
     av_free(audio_element->substreams);
     av_free(audio_element->layers);
     av_iamf_audio_element_free(&audio_element->element);
-- 
2.45.2

_______________________________________________
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] [PATCH 5/5] avformat/iamf: Check substreams in ff_iamf_free_audio_element()

[Michael Niedermayer]

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

On Mon, Jun 24, 2024 at 01:01:37AM +0200, Michael Niedermayer wrote:
> Fixes: member access within null pointer of type 'IAMFSubStream' (aka 'struct IAMFSubStream')
> Fixes: 69795/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6216287009701888
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/iamf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

will apply

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"

[-- 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] [PATCH 5/5] avformat/iamf: Check substreams in ff_iamf_free_audio_element()

[James Almer]

On 6/23/2024 8:01 PM, Michael Niedermayer wrote:
> Fixes: member access within null pointer of type 'IAMFSubStream' (aka 'struct IAMFSubStream')
> Fixes: 69795/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6216287009701888
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/iamf.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/iamf.c b/libavformat/iamf.c
> index 5de70dc0828..364cb60e021 100644
> --- a/libavformat/iamf.c
> +++ b/libavformat/iamf.c
> @@ -74,8 +74,10 @@ void ff_iamf_free_audio_element(IAMFAudioElement **paudio_element)
>       if (!audio_element)
>           return;
>   
> -    for (int i = 0; i < audio_element->nb_substreams; i++)
> -        avcodec_parameters_free(&audio_element->substreams[i].codecpar);
> +    if (audio_element->nb_substreams)
> +        for (int i = 0; i < audio_element->nb_substreams; i++) {
> +            avcodec_parameters_free(&audio_element->substreams[i].codecpar);
> +        }
>       av_free(audio_element->substreams);
>       av_free(audio_element->layers);
>       av_iamf_audio_element_free(&audio_element->element);

Sorry, i missed this.

nb_substreams shouldn't be anything but 0 here if nb_substreams is NULL, 
so the following is IMO better:

> diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
> index 9cec12d46f..a69d4a2f3a 100644
> --- a/libavformat/iamf_parse.c
> +++ b/libavformat/iamf_parse.c
> @@ -594,7 +594,7 @@ static int audio_element_obu(void *s, IAMFContext *c, AVIOContext *pb, int len)
>      FFIOContext b;
>      AVIOContext *pbc;
>      uint8_t *buf;
> -    unsigned audio_element_id, codec_config_id, num_parameters;
> +    unsigned audio_element_id, nb_substreams, codec_config_id, num_parameters;
>      int audio_element_type, ret;
> 
>      buf = av_malloc(len);
> @@ -649,14 +649,15 @@ static int audio_element_obu(void *s, IAMFContext *c, AVIOContext *pb, int len)
>          goto fail;
>      }
> 
> -    audio_element->nb_substreams = ffio_read_leb(pbc);
> +    nb_substreams = ffio_read_leb(pbc);
>      audio_element->codec_config_id = codec_config_id;
>      audio_element->audio_element_id = audio_element_id;
> -    audio_element->substreams = av_calloc(audio_element->nb_substreams, sizeof(*audio_element->substreams));
> +    audio_element->substreams = av_calloc(nb_substreams, sizeof(*audio_element->substreams));
>      if (!audio_element->substreams) {
>          ret = AVERROR(ENOMEM);
>          goto fail;
>      }
> +    audio_element->nb_substreams = nb_substreams;
> 
>      element = audio_element->element = av_iamf_audio_element_alloc();
>      if (!element) {
_______________________________________________
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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1476 bytes --]

On 24/06/2024 01:01, Michael Niedermayer wrote:
> ff_aac_usac_config_decode() needs AACDecContext to be set but some callers
> pass NULL.
> 
> I have no real testcase to implement/test this, so failing in this case
> seems safest.
> 
> Fixes: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
> Fixes: 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/aac/aacdec_usac.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
> index 132ffee9c20..4856c1786b7 100644
> --- a/libavcodec/aac/aacdec_usac.c
> +++ b/libavcodec/aac/aacdec_usac.c
> @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac, AVCodecContext *avctx,
>       int map_pos_set = 0;
>       uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
>   
> +    if (!ac)
> +        return AVERROR_PATCHWELCOME;
> +
>       memset(usac, 0, sizeof(*usac));
>   
>       freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */

This doesn't seem possible at all.
There are 2 callers, parse_audio_preroll and 
decode_audio_specific_config_gb. Both of these will crash way before the 
function is called.

Could you at least get a backtrace?

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Michael Niedermayer]

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

On Wed, Jun 26, 2024 at 01:35:18AM +0200, Lynne via ffmpeg-devel wrote:
> On 24/06/2024 01:01, Michael Niedermayer wrote:
> > ff_aac_usac_config_decode() needs AACDecContext to be set but some callers
> > pass NULL.
> > 
> > I have no real testcase to implement/test this, so failing in this case
> > seems safest.
> > 
> > Fixes: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
> > Fixes: 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/aac/aacdec_usac.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
> > index 132ffee9c20..4856c1786b7 100644
> > --- a/libavcodec/aac/aacdec_usac.c
> > +++ b/libavcodec/aac/aacdec_usac.c
> > @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac, AVCodecContext *avctx,
> >       int map_pos_set = 0;
> >       uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
> > +    if (!ac)
> > +        return AVERROR_PATCHWELCOME;
> > +
> >       memset(usac, 0, sizeof(*usac));
> >       freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
> 
> This doesn't seem possible at all.
> There are 2 callers, parse_audio_preroll and
> decode_audio_specific_config_gb. Both of these will crash way before the
> function is called.
> 
> Could you at least get a backtrace?

sure

libavcodec/aac/aacdec_usac.c:402:39: runtime error: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/aac/aacdec_usac.c:402:39 in
AddressSanitizer:DEADLYSIGNAL
=================================================================
==215602==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005b10 (pc 0x00000154771e bp 0x7ffcf5049e90 sp 0x7ffcf5049e70 T0)
==215602==The signal is caused by a READ memory access.
    #0 0x154771d in av_channel_layout_uninit ffmpeg/libavutil/channel_layout.c:439:25
    #1 0x57346e in ff_aac_usac_config_decode ffmpeg/libavcodec/aac/aacdec_usac.c:402:9
    #2 0x500a0a in decode_audio_specific_config_gb ffmpeg/libavcodec/aac/aacdec.c:1050:20
    #3 0x50a542 in latm_decode_audio_specific_config ffmpeg/libavcodec/aac/aacdec_latm.h:80:21
    #4 0x4f8638 in read_stream_mux_config ffmpeg/libavcodec/aac/aacdec_latm.h:160:24
    #5 0x4f8638 in read_audio_mux_element ffmpeg/libavcodec/aac/aacdec_latm.h:233
    #6 0x4f8638 in latm_decode_frame ffmpeg/libavcodec/aac/aacdec_latm.h:275
    #7 0x68f26f in decode_simple_internal ffmpeg/libavcodec/decode.c:429:20
    #8 0x68f26f in decode_simple_receive_frame ffmpeg/libavcodec/decode.c:600
    #9 0x68f26f in decode_receive_frame_internal ffmpeg/libavcodec/decode.c:631
    #10 0x68dc9d in avcodec_send_packet ffmpeg/libavcodec/decode.c:721:15
    #11 0x4d1e55 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dec_fuzzer.c:534:25


[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.


[-- 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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 3742 bytes --]

On 26/06/2024 01:57, Michael Niedermayer wrote:
> On Wed, Jun 26, 2024 at 01:35:18AM +0200, Lynne via ffmpeg-devel wrote:
>> On 24/06/2024 01:01, Michael Niedermayer wrote:
>>> ff_aac_usac_config_decode() needs AACDecContext to be set but some callers
>>> pass NULL.
>>>
>>> I have no real testcase to implement/test this, so failing in this case
>>> seems safest.
>>>
>>> Fixes: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
>>> Fixes: 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavcodec/aac/aacdec_usac.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
>>> index 132ffee9c20..4856c1786b7 100644
>>> --- a/libavcodec/aac/aacdec_usac.c
>>> +++ b/libavcodec/aac/aacdec_usac.c
>>> @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac, AVCodecContext *avctx,
>>>        int map_pos_set = 0;
>>>        uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
>>> +    if (!ac)
>>> +        return AVERROR_PATCHWELCOME;
>>> +
>>>        memset(usac, 0, sizeof(*usac));
>>>        freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
>>
>> This doesn't seem possible at all.
>> There are 2 callers, parse_audio_preroll and
>> decode_audio_specific_config_gb. Both of these will crash way before the
>> function is called.
>>
>> Could you at least get a backtrace?
> 
> sure
> 
> libavcodec/aac/aacdec_usac.c:402:39: runtime error: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/aac/aacdec_usac.c:402:39 in
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==215602==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005b10 (pc 0x00000154771e bp 0x7ffcf5049e90 sp 0x7ffcf5049e70 T0)
> ==215602==The signal is caused by a READ memory access.
>      #0 0x154771d in av_channel_layout_uninit ffmpeg/libavutil/channel_layout.c:439:25
>      #1 0x57346e in ff_aac_usac_config_decode ffmpeg/libavcodec/aac/aacdec_usac.c:402:9
>      #2 0x500a0a in decode_audio_specific_config_gb ffmpeg/libavcodec/aac/aacdec.c:1050:20
>      #3 0x50a542 in latm_decode_audio_specific_config ffmpeg/libavcodec/aac/aacdec_latm.h:80:21
>      #4 0x4f8638 in read_stream_mux_config ffmpeg/libavcodec/aac/aacdec_latm.h:160:24
>      #5 0x4f8638 in read_audio_mux_element ffmpeg/libavcodec/aac/aacdec_latm.h:233
>      #6 0x4f8638 in latm_decode_frame ffmpeg/libavcodec/aac/aacdec_latm.h:275
>      #7 0x68f26f in decode_simple_internal ffmpeg/libavcodec/decode.c:429:20
>      #8 0x68f26f in decode_simple_receive_frame ffmpeg/libavcodec/decode.c:600
>      #9 0x68f26f in decode_receive_frame_internal ffmpeg/libavcodec/decode.c:631
>      #10 0x68dc9d in avcodec_send_packet ffmpeg/libavcodec/decode.c:721:15
>      #11 0x4d1e55 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dec_fuzzer.c:534:25
> 
> 
> [...]
> 
> 
> 
> _______________________________________________
> 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".

This patch actually is correct.
USAC in LATM is not supported AFAIK.

LGTM with a note like:
"Happens only when the LATM decoder is used, and USAC is not supported 
in LATM".

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Michael Niedermayer]

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

On Wed, Jun 26, 2024 at 08:58:27AM +0200, Lynne via ffmpeg-devel wrote:
> On 26/06/2024 01:57, Michael Niedermayer wrote:
> > On Wed, Jun 26, 2024 at 01:35:18AM +0200, Lynne via ffmpeg-devel wrote:
> > > On 24/06/2024 01:01, Michael Niedermayer wrote:
> > > > ff_aac_usac_config_decode() needs AACDecContext to be set but some callers
> > > > pass NULL.
> > > > 
> > > > I have no real testcase to implement/test this, so failing in this case
> > > > seems safest.
> > > > 
> > > > Fixes: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
> > > > Fixes: 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
> > > > 
> > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavcodec/aac/aacdec_usac.c | 3 +++
> > > >    1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
> > > > index 132ffee9c20..4856c1786b7 100644
> > > > --- a/libavcodec/aac/aacdec_usac.c
> > > > +++ b/libavcodec/aac/aacdec_usac.c
> > > > @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac, AVCodecContext *avctx,
> > > >        int map_pos_set = 0;
> > > >        uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
> > > > +    if (!ac)
> > > > +        return AVERROR_PATCHWELCOME;
> > > > +
> > > >        memset(usac, 0, sizeof(*usac));
> > > >        freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
> > > 
> > > This doesn't seem possible at all.
> > > There are 2 callers, parse_audio_preroll and
> > > decode_audio_specific_config_gb. Both of these will crash way before the
> > > function is called.
> > > 
> > > Could you at least get a backtrace?
> > 
> > sure
> > 
> > libavcodec/aac/aacdec_usac.c:402:39: runtime error: member access within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/aac/aacdec_usac.c:402:39 in
> > AddressSanitizer:DEADLYSIGNAL
> > =================================================================
> > ==215602==ERROR: AddressSanitizer: SEGV on unknown address 0x000000005b10 (pc 0x00000154771e bp 0x7ffcf5049e90 sp 0x7ffcf5049e70 T0)
> > ==215602==The signal is caused by a READ memory access.
> >      #0 0x154771d in av_channel_layout_uninit ffmpeg/libavutil/channel_layout.c:439:25
> >      #1 0x57346e in ff_aac_usac_config_decode ffmpeg/libavcodec/aac/aacdec_usac.c:402:9
> >      #2 0x500a0a in decode_audio_specific_config_gb ffmpeg/libavcodec/aac/aacdec.c:1050:20
> >      #3 0x50a542 in latm_decode_audio_specific_config ffmpeg/libavcodec/aac/aacdec_latm.h:80:21
> >      #4 0x4f8638 in read_stream_mux_config ffmpeg/libavcodec/aac/aacdec_latm.h:160:24
> >      #5 0x4f8638 in read_audio_mux_element ffmpeg/libavcodec/aac/aacdec_latm.h:233
> >      #6 0x4f8638 in latm_decode_frame ffmpeg/libavcodec/aac/aacdec_latm.h:275
> >      #7 0x68f26f in decode_simple_internal ffmpeg/libavcodec/decode.c:429:20
> >      #8 0x68f26f in decode_simple_receive_frame ffmpeg/libavcodec/decode.c:600
> >      #9 0x68f26f in decode_receive_frame_internal ffmpeg/libavcodec/decode.c:631
> >      #10 0x68dc9d in avcodec_send_packet ffmpeg/libavcodec/decode.c:721:15
> >      #11 0x4d1e55 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dec_fuzzer.c:534:25
> > 
> > 
> > [...]
> > 
> > 
> > 
> > _______________________________________________
> > 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".
> 
> This patch actually is correct.
> USAC in LATM is not supported AFAIK.
> 
> LGTM with a note like:
> "Happens only when the LATM decoder is used, and USAC is not supported in
> LATM".

will apply with this

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.

[-- 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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Andreas Rheinhardt]

Lynne via ffmpeg-devel:
> On 26/06/2024 01:57, Michael Niedermayer wrote:
>> On Wed, Jun 26, 2024 at 01:35:18AM +0200, Lynne via ffmpeg-devel wrote:
>>> On 24/06/2024 01:01, Michael Niedermayer wrote:
>>>> ff_aac_usac_config_decode() needs AACDecContext to be set but some
>>>> callers
>>>> pass NULL.
>>>>
>>>> I have no real testcase to implement/test this, so failing in this case
>>>> seems safest.
>>>>
>>>> Fixes: member access within null pointer of type 'AACDecContext'
>>>> (aka 'struct AACDecContext')
>>>> Fixes:
>>>> 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
>>>>
>>>> Found-by: continuous fuzzing process
>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>>> ---
>>>>    libavcodec/aac/aacdec_usac.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/libavcodec/aac/aacdec_usac.c
>>>> b/libavcodec/aac/aacdec_usac.c
>>>> index 132ffee9c20..4856c1786b7 100644
>>>> --- a/libavcodec/aac/aacdec_usac.c
>>>> +++ b/libavcodec/aac/aacdec_usac.c
>>>> @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac,
>>>> AVCodecContext *avctx,
>>>>        int map_pos_set = 0;
>>>>        uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
>>>> +    if (!ac)
>>>> +        return AVERROR_PATCHWELCOME;
>>>> +
>>>>        memset(usac, 0, sizeof(*usac));
>>>>        freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
>>>
>>> This doesn't seem possible at all.
>>> There are 2 callers, parse_audio_preroll and
>>> decode_audio_specific_config_gb. Both of these will crash way before the
>>> function is called.
>>>
>>> Could you at least get a backtrace?
>>
>> sure
>>
>> libavcodec/aac/aacdec_usac.c:402:39: runtime error: member access
>> within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
>> libavcodec/aac/aacdec_usac.c:402:39 in
>> AddressSanitizer:DEADLYSIGNAL
>> =================================================================
>> ==215602==ERROR: AddressSanitizer: SEGV on unknown address
>> 0x000000005b10 (pc 0x00000154771e bp 0x7ffcf5049e90 sp 0x7ffcf5049e70 T0)
>> ==215602==The signal is caused by a READ memory access.
>>      #0 0x154771d in av_channel_layout_uninit
>> ffmpeg/libavutil/channel_layout.c:439:25
>>      #1 0x57346e in ff_aac_usac_config_decode
>> ffmpeg/libavcodec/aac/aacdec_usac.c:402:9
>>      #2 0x500a0a in decode_audio_specific_config_gb
>> ffmpeg/libavcodec/aac/aacdec.c:1050:20
>>      #3 0x50a542 in latm_decode_audio_specific_config
>> ffmpeg/libavcodec/aac/aacdec_latm.h:80:21
>>      #4 0x4f8638 in read_stream_mux_config
>> ffmpeg/libavcodec/aac/aacdec_latm.h:160:24
>>      #5 0x4f8638 in read_audio_mux_element
>> ffmpeg/libavcodec/aac/aacdec_latm.h:233
>>      #6 0x4f8638 in latm_decode_frame
>> ffmpeg/libavcodec/aac/aacdec_latm.h:275
>>      #7 0x68f26f in decode_simple_internal
>> ffmpeg/libavcodec/decode.c:429:20
>>      #8 0x68f26f in decode_simple_receive_frame
>> ffmpeg/libavcodec/decode.c:600
>>      #9 0x68f26f in decode_receive_frame_internal
>> ffmpeg/libavcodec/decode.c:631
>>      #10 0x68dc9d in avcodec_send_packet
>> ffmpeg/libavcodec/decode.c:721:15
>>      #11 0x4d1e55 in LLVMFuzzerTestOneInput
>> ffmpeg/tools/target_dec_fuzzer.c:534:25
>>
>>
>> [...]
>>
>>
>>
>> _______________________________________________
>> 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".
> 
> This patch actually is correct.
> USAC in LATM is not supported AFAIK.
> 
> LGTM with a note like:
> "Happens only when the LATM decoder is used, and USAC is not supported
> in LATM".
> 

But then it is not PATCHWELCOME, but INVALIDDATA.

- Andreas

_______________________________________________
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] [PATCH 1/5] avcodec/aac/aacdec_usac: Test ac in usac

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 4402 bytes --]

On 26/06/2024 21:45, Andreas Rheinhardt wrote:
> Lynne via ffmpeg-devel:
>> On 26/06/2024 01:57, Michael Niedermayer wrote:
>>> On Wed, Jun 26, 2024 at 01:35:18AM +0200, Lynne via ffmpeg-devel wrote:
>>>> On 24/06/2024 01:01, Michael Niedermayer wrote:
>>>>> ff_aac_usac_config_decode() needs AACDecContext to be set but some
>>>>> callers
>>>>> pass NULL.
>>>>>
>>>>> I have no real testcase to implement/test this, so failing in this case
>>>>> seems safest.
>>>>>
>>>>> Fixes: member access within null pointer of type 'AACDecContext'
>>>>> (aka 'struct AACDecContext')
>>>>> Fixes:
>>>>> 69435/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-5733527483121664
>>>>>
>>>>> Found-by: continuous fuzzing process
>>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>>>> ---
>>>>>     libavcodec/aac/aacdec_usac.c | 3 +++
>>>>>     1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/libavcodec/aac/aacdec_usac.c
>>>>> b/libavcodec/aac/aacdec_usac.c
>>>>> index 132ffee9c20..4856c1786b7 100644
>>>>> --- a/libavcodec/aac/aacdec_usac.c
>>>>> +++ b/libavcodec/aac/aacdec_usac.c
>>>>> @@ -348,6 +348,9 @@ int ff_aac_usac_config_decode(AACDecContext *ac,
>>>>> AVCodecContext *avctx,
>>>>>         int map_pos_set = 0;
>>>>>         uint8_t layout_map[MAX_ELEM_ID*4][3] = { 0 };
>>>>> +    if (!ac)
>>>>> +        return AVERROR_PATCHWELCOME;
>>>>> +
>>>>>         memset(usac, 0, sizeof(*usac));
>>>>>         freq_idx = get_bits(gb, 5); /* usacSamplingFrequencyIndex */
>>>>
>>>> This doesn't seem possible at all.
>>>> There are 2 callers, parse_audio_preroll and
>>>> decode_audio_specific_config_gb. Both of these will crash way before the
>>>> function is called.
>>>>
>>>> Could you at least get a backtrace?
>>>
>>> sure
>>>
>>> libavcodec/aac/aacdec_usac.c:402:39: runtime error: member access
>>> within null pointer of type 'AACDecContext' (aka 'struct AACDecContext')
>>> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
>>> libavcodec/aac/aacdec_usac.c:402:39 in
>>> AddressSanitizer:DEADLYSIGNAL
>>> =================================================================
>>> ==215602==ERROR: AddressSanitizer: SEGV on unknown address
>>> 0x000000005b10 (pc 0x00000154771e bp 0x7ffcf5049e90 sp 0x7ffcf5049e70 T0)
>>> ==215602==The signal is caused by a READ memory access.
>>>       #0 0x154771d in av_channel_layout_uninit
>>> ffmpeg/libavutil/channel_layout.c:439:25
>>>       #1 0x57346e in ff_aac_usac_config_decode
>>> ffmpeg/libavcodec/aac/aacdec_usac.c:402:9
>>>       #2 0x500a0a in decode_audio_specific_config_gb
>>> ffmpeg/libavcodec/aac/aacdec.c:1050:20
>>>       #3 0x50a542 in latm_decode_audio_specific_config
>>> ffmpeg/libavcodec/aac/aacdec_latm.h:80:21
>>>       #4 0x4f8638 in read_stream_mux_config
>>> ffmpeg/libavcodec/aac/aacdec_latm.h:160:24
>>>       #5 0x4f8638 in read_audio_mux_element
>>> ffmpeg/libavcodec/aac/aacdec_latm.h:233
>>>       #6 0x4f8638 in latm_decode_frame
>>> ffmpeg/libavcodec/aac/aacdec_latm.h:275
>>>       #7 0x68f26f in decode_simple_internal
>>> ffmpeg/libavcodec/decode.c:429:20
>>>       #8 0x68f26f in decode_simple_receive_frame
>>> ffmpeg/libavcodec/decode.c:600
>>>       #9 0x68f26f in decode_receive_frame_internal
>>> ffmpeg/libavcodec/decode.c:631
>>>       #10 0x68dc9d in avcodec_send_packet
>>> ffmpeg/libavcodec/decode.c:721:15
>>>       #11 0x4d1e55 in LLVMFuzzerTestOneInput
>>> ffmpeg/tools/target_dec_fuzzer.c:534:25
>>>
>>>
>>> [...]
>>>
>>>
>>>
>>> _______________________________________________
>>> 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".
>>
>> This patch actually is correct.
>> USAC in LATM is not supported AFAIK.
>>
>> LGTM with a note like:
>> "Happens only when the LATM decoder is used, and USAC is not supported
>> in LATM".
>>
> 
> But then it is not PATCHWELCOME, but INVALIDDATA.

I got an email from one of the developers who informed me that its 
supported in LATM.
But, I have no samples, so patch welcome is fine for now.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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".