[FFmpeg-devel] [PATCH 1/8] avcodec/dnxuc_parser: Use ff_parse_close()

[FFmpeg-devel] [PATCH 1/8] avcodec/dnxuc_parser: Use ff_parse_close()

[Michael Niedermayer]

Fixes: buffer leak
Fixes: 398894512/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6716597473705984

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

diff --git a/libavcodec/dnxuc_parser.c b/libavcodec/dnxuc_parser.c
index 14f19efe676..34088ac3b1c 100644
--- a/libavcodec/dnxuc_parser.c
+++ b/libavcodec/dnxuc_parser.c
@@ -85,4 +85,5 @@ const AVCodecParser ff_dnxuc_parser = {
     .codec_ids      = { AV_CODEC_ID_DNXUC },
     .priv_data_size = sizeof(DNxUcParseContext),
     .parser_parse   = dnxuc_parse,
+    .parser_close   = ff_parse_close,
 };
-- 
2.49.0

_______________________________________________
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/8] avformat/mov: reject negative ELST durations

[Michael Niedermayer]

Fixes: multiple integer overflows
Fixes: 401016767/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-6242067591790592

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

diff --git a/libavformat/mov.c b/libavformat/mov.c
index cb81b6c4da8..8a094b1ea0a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6295,6 +6295,11 @@ static int mov_read_elst(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                    c->fc->nb_streams-1, i, e->time);
             return AVERROR_INVALIDDATA;
         }
+        if (e->duration < 0) {
+            av_log(c->fc, AV_LOG_ERROR, "Track %d, edit %d: Invalid edit list duration=%"PRId64"\n",
+                   c->fc->nb_streams-1, i, e->duration);
+            return AVERROR_INVALIDDATA;
+        }
     }
     sc->elst_count = i;
 
-- 
2.49.0

_______________________________________________
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/8] avformat/imf_cpl: do not continue looping forever

[Michael Niedermayer]

Fixes: infinite loop
Fixes: 401658595/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5756875014733824
Regression since: 61fa1e14e4178d3f2550c76f7a36484220f6dc0c

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

diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
index 137cff2a63c..6ddea62abef 100644
--- a/libavformat/imf_cpl.c
+++ b/libavformat/imf_cpl.c
@@ -709,8 +709,7 @@ static int fill_virtual_tracks(void *log_ctx, xmlNodePtr cpl_element, FFIMFCPL *
         av_log(log_ctx, AV_LOG_DEBUG, "Processing IMF CPL Segment\n");
 
         sequence_list_elem = ff_imf_xml_get_child_element_by_name(segment_elem, "SequenceList");
-        if (!sequence_list_elem)
-            continue;
+        if (sequence_list_elem) {
 
         sequence_elem = xmlFirstElementChild(sequence_list_elem);
         while (sequence_elem) {
@@ -735,6 +734,7 @@ static int fill_virtual_tracks(void *log_ctx, xmlNodePtr cpl_element, FFIMFCPL *
 
             sequence_elem = xmlNextElementSibling(sequence_elem);
         }
+        }
 
         segment_elem = xmlNextElementSibling(segment_elem);
     }
-- 
2.49.0

_______________________________________________
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/8] avformat/imf_cpl: fix indention after previous commit

[Michael Niedermayer]

---
 libavformat/imf_cpl.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
index 6ddea62abef..8c3530f4122 100644
--- a/libavformat/imf_cpl.c
+++ b/libavformat/imf_cpl.c
@@ -711,29 +711,29 @@ static int fill_virtual_tracks(void *log_ctx, xmlNodePtr cpl_element, FFIMFCPL *
         sequence_list_elem = ff_imf_xml_get_child_element_by_name(segment_elem, "SequenceList");
         if (sequence_list_elem) {
 
-        sequence_elem = xmlFirstElementChild(sequence_list_elem);
-        while (sequence_elem) {
-            if (xmlStrcmp(sequence_elem->name, "MarkerSequence") == 0)
-                ret = push_marker_sequence(log_ctx, sequence_elem, cpl);
+            sequence_elem = xmlFirstElementChild(sequence_list_elem);
+            while (sequence_elem) {
+                if (xmlStrcmp(sequence_elem->name, "MarkerSequence") == 0)
+                    ret = push_marker_sequence(log_ctx, sequence_elem, cpl);
 
-            else if (xmlStrcmp(sequence_elem->name, "MainImageSequence") == 0)
-                ret = push_main_image_2d_sequence(log_ctx, sequence_elem, cpl);
+                else if (xmlStrcmp(sequence_elem->name, "MainImageSequence") == 0)
+                    ret = push_main_image_2d_sequence(log_ctx, sequence_elem, cpl);
 
-            else if (xmlStrcmp(sequence_elem->name, "MainAudioSequence") == 0)
-                ret = push_main_audio_sequence(log_ctx, sequence_elem, cpl);
+                else if (xmlStrcmp(sequence_elem->name, "MainAudioSequence") == 0)
+                    ret = push_main_audio_sequence(log_ctx, sequence_elem, cpl);
 
-            else
-                av_log(log_ctx,
-                       AV_LOG_INFO,
-                       "The following Sequence is not supported and is ignored: %s\n",
-                       sequence_elem->name);
+                else
+                    av_log(log_ctx,
+                        AV_LOG_INFO,
+                        "The following Sequence is not supported and is ignored: %s\n",
+                        sequence_elem->name);
 
-            /* abort parsing only if memory error occurred */
-            if (ret == AVERROR(ENOMEM))
-                return ret;
+                /* abort parsing only if memory error occurred */
+                if (ret == AVERROR(ENOMEM))
+                    return ret;
 
-            sequence_elem = xmlNextElementSibling(sequence_elem);
-        }
+                sequence_elem = xmlNextElementSibling(sequence_elem);
+            }
         }
 
         segment_elem = xmlNextElementSibling(segment_elem);
-- 
2.49.0

_______________________________________________
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/8] avcodec/aacsbr_template: Check ilb

[Michael Niedermayer]

Fixes: index 50 out of bounds for type 'INTFLOAT [40][2]'
Fixes: 401661737/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4866055713652736

Someone knowing AAC should review this, there is likely a nicer fix

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

diff --git a/libavcodec/aacsbr_template.c b/libavcodec/aacsbr_template.c
index 9fae44d9a5c..a126bd6f060 100644
--- a/libavcodec/aacsbr_template.c
+++ b/libavcodec/aacsbr_template.c
@@ -1626,6 +1626,9 @@ static void sbr_env_estimate(AAC_FLOAT (*e_curr)[48], INTFLOAT X_high[64][40][2]
             int ilb = ch_data->t_env[e]     * 2 + ENVELOPE_ADJUSTMENT_OFFSET;
             int iub = ch_data->t_env[e + 1] * 2 + ENVELOPE_ADJUSTMENT_OFFSET;
 
+            if (ilb >= 40)
+                return;
+
             for (m = 0; m < sbr->m[1]; m++) {
                 AAC_FLOAT sum = sbr->dsp.sum_square(X_high[m+kx1] + ilb, iub - ilb);
 #if USE_FIXED
@@ -1644,6 +1647,9 @@ static void sbr_env_estimate(AAC_FLOAT (*e_curr)[48], INTFLOAT X_high[64][40][2]
             int iub = ch_data->t_env[e + 1] * 2 + ENVELOPE_ADJUSTMENT_OFFSET;
             const uint16_t *table = ch_data->bs_freq_res[e + 1] ? sbr->f_tablehigh : sbr->f_tablelow;
 
+            if (ilb >= 40)
+                return;
+
             for (p = 0; p < sbr->n[ch_data->bs_freq_res[e + 1]]; p++) {
 #if USE_FIXED
                 SoftFloat sum = FLOAT_0;
-- 
2.49.0

_______________________________________________
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 6/8] avcodec/sonic: Check num_taps

[Michael Niedermayer]

The encoder uses max 128 taps, which is quiet a lot already
If work is done to improve sonic, it will be more radical than changing the taps

Fixes: Timeout
Fixes: 402539974/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SONIC_fuzzer-6122944271286272

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

diff --git a/libavcodec/sonic.c b/libavcodec/sonic.c
index 60b077de47d..8b1d092ec91 100644
--- a/libavcodec/sonic.c
+++ b/libavcodec/sonic.c
@@ -924,6 +924,9 @@ static av_cold int sonic_decode_init(AVCodecContext *avctx)
     if (get_bits1(&gb)) // XXX FIXME
         av_log(avctx, AV_LOG_INFO, "Custom quant table\n");
 
+    if (s->num_taps > 128)
+        return AVERROR_INVALIDDATA;
+
     s->block_align = 2048LL*s->samplerate/(44100*s->downsampling);
     s->frame_size = s->channels*s->block_align*s->downsampling;
 //    avctx->frame_size = s->block_align;
-- 
2.49.0

_______________________________________________
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 7/8] avcodec/svq3: Check that for 8 byte space before subtracting

[Michael Niedermayer]

No testcase

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/svq3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
index f730358e2f9..30bc9334af7 100644
--- a/libavcodec/svq3.c
+++ b/libavcodec/svq3.c
@@ -1173,7 +1173,7 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
         int w,h;
 
         size = AV_RB32(&extradata[4]);
-        if (size > extradata_end - extradata - 8)
+        if (extradata_end - extradata < 8 || size > extradata_end - extradata - 8)
             return AVERROR_INVALIDDATA;
         init_get_bits(&gb, extradata + 8, size * 8);
 
-- 
2.49.0

_______________________________________________
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 8/8] avcodec/svq3: Check there are bits left before decompression

[Michael Niedermayer]

Fixes: out of array read
Fixes: 402587670/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SVQ3_fuzzer-6343867775647744

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

diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
index 30bc9334af7..7a95aa4cff9 100644
--- a/libavcodec/svq3.c
+++ b/libavcodec/svq3.c
@@ -1253,6 +1253,7 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
             uint8_t *buf;
 
             if (watermark_height <= 0 ||
+                get_bits_left(&gb) <= 0 ||
                 (uint64_t)watermark_width * 4 > UINT_MAX / watermark_height)
                 return AVERROR_INVALIDDATA;
 
-- 
2.49.0

_______________________________________________
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 7/8] avcodec/svq3: Check that for 8 byte space before subtracting

[Andreas Rheinhardt]

Michael Niedermayer:
> No testcase
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/svq3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
> index f730358e2f9..30bc9334af7 100644
> --- a/libavcodec/svq3.c
> +++ b/libavcodec/svq3.c
> @@ -1173,7 +1173,7 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
>          int w,h;
>  
>          size = AV_RB32(&extradata[4]);
> -        if (size > extradata_end - extradata - 8)
> +        if (extradata_end - extradata < 8 || size > extradata_end - extradata - 8)
>              return AVERROR_INVALIDDATA;
>          init_get_bits(&gb, extradata + 8, size * 8);
>  

Can't be triggered: This code is only executed iff marker_found is 1;
and given the "m + 8 < avctx->extradata_size" check in the loop it is
guaranteed that there are at least eight bytes of extradata available.

- 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 7/8] avcodec/svq3: Check that for 8 byte space before subtracting

[Michael Niedermayer]

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

On Wed, May 14, 2025 at 06:34:25PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > No testcase
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/svq3.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
> > index f730358e2f9..30bc9334af7 100644
> > --- a/libavcodec/svq3.c
> > +++ b/libavcodec/svq3.c
> > @@ -1173,7 +1173,7 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
> >          int w,h;
> >  
> >          size = AV_RB32(&extradata[4]);
> > -        if (size > extradata_end - extradata - 8)
> > +        if (extradata_end - extradata < 8 || size > extradata_end - extradata - 8)
> >              return AVERROR_INVALIDDATA;
> >          init_get_bits(&gb, extradata + 8, size * 8);
> >  
> 
> Can't be triggered: This code is only executed iff marker_found is 1;
> and given the "m + 8 < avctx->extradata_size" check in the loop it is
> guaranteed that there are at least eight bytes of extradata available.

True

Did we ever had someone miss such distributed checks and
produce buggy code through a change ?
If not then i think you are correct here and lets skip adding an
explicit check, its ugly to have such redundant checks

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

[-- 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/8] avcodec/dnxuc_parser: Use ff_parse_close()

[Michael Niedermayer]

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

On Sun, May 11, 2025 at 02:32:38AM +0200, Michael Niedermayer wrote:
> Fixes: buffer leak
> Fixes: 398894512/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6716597473705984
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/dnxuc_parser.c | 1 +
>  1 file changed, 1 insertion(+)

will apply patchset except 5 and 7


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

z(9) = an object that transcends all computable functions describable
in finite terms. - ChatGPT in 2024

[-- 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 7/8] avcodec/svq3: Check that for 8 byte space before subtracting

[Andreas Rheinhardt]

Michael Niedermayer:
> On Wed, May 14, 2025 at 06:34:25PM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> No testcase
>>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>  libavcodec/svq3.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
>>> index f730358e2f9..30bc9334af7 100644
>>> --- a/libavcodec/svq3.c
>>> +++ b/libavcodec/svq3.c
>>> @@ -1173,7 +1173,7 @@ static av_cold int svq3_decode_init(AVCodecContext *avctx)
>>>          int w,h;
>>>  
>>>          size = AV_RB32(&extradata[4]);
>>> -        if (size > extradata_end - extradata - 8)
>>> +        if (extradata_end - extradata < 8 || size > extradata_end - extradata - 8)
>>>              return AVERROR_INVALIDDATA;
>>>          init_get_bits(&gb, extradata + 8, size * 8);
>>>  
>>
>> Can't be triggered: This code is only executed iff marker_found is 1;
>> and given the "m + 8 < avctx->extradata_size" check in the loop it is
>> guaranteed that there are at least eight bytes of extradata available.
> 
> True
> 
> Did we ever had someone miss such distributed checks and
> produce buggy code through a change ?
> If not then i think you are correct here and lets skip adding an
> explicit check, its ugly to have such redundant checks
> 

We could avoid the whole marker_found branch (and the variable) by
moving the whole if (marker_found) block into a function of its own that
is called where currently marker_found is set to one. I'll send a patch
for this.

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