[PATCH 1/8] avcodec/dnxuc_parser: Use ff_parse_close()
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
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
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
--- 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
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
On Sun, May 11, 2025 at 02:32:42AM +0200, Michael Niedermayer wrote:
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
noone :( will apply this then thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Take away the freedom of one citizen and you will be jailed, take away the freedom of all citizens and you will be congratulated by your peers in Parliament.
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
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
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
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
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
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
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
participants (2)
-
Andreas Rheinhardt -
Michael Niedermayer