[PR] avformat/iamf_parse: check that Parameter Definitions subblocks values are valid (PR #23915)

PR #23915 opened by James Almer (jamrial)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23915
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23915.patch

Do a dry run of the subblocks structure before allocating an AVIAMFParamDefinition.


>From 0ab7813be9c6943c653e3e22d9a702c5fb8a402a Mon Sep 17 00:00:00 2001
From: James Almer <[email protected]>
Date: Sat, 25 Jul 2026 12:20:22 -0300
Subject: [PATCH 1/3] avformat/iamf_parse: check that num_sub_mixes and
 num_audio_elements in Mix Presentations are not zero

As required by the spec in Section 3.7

Signed-off-by: James Almer <[email protected]>
---
 libavformat/iamf_parse.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
index 4c2df2c9e6..90bccdb6f9 100644
--- a/libavformat/iamf_parse.c
+++ b/libavformat/iamf_parse.c
@@ -1040,6 +1040,12 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le
     }
 
     nb_submixes = ffio_read_leb(pbc);
+    if (!nb_submixes) {
+        av_log(s, AV_LOG_ERROR, "Mix presentation %u has no submixes\n", mix_presentation_id);
+        ret = AVERROR_INVALIDDATA;
+        goto fail;
+    }
+
     for (int i = 0; i < nb_submixes; i++) {
         AVIAMFSubmix *sub_mix;
         unsigned nb_elements, nb_layouts;
@@ -1051,6 +1057,13 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le
         }
 
         nb_elements = ffio_read_leb(pbc);
+        if (!nb_elements) {
+            av_log(s, AV_LOG_ERROR, "Submix %d from Mix presentation %u has no audio elements\n",
+                   i, mix_presentation_id);
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
         for (int j = 0; j < nb_elements; j++) {
             AVIAMFSubmixElement *submix_element;
             IAMFAudioElement *audio_element = NULL;
-- 
2.52.0


>From f08aec09cd5f197799db0ce0f98d8d561e9affe1 Mon Sep 17 00:00:00 2001
From: James Almer <[email protected]>
Date: Sat, 25 Jul 2026 12:30:23 -0300
Subject: [PATCH 2/3] libavformat/iamf: don't bound parameter definition
 durations to audio element frame durations

They could define parameters that cover the entire stream, not a single frame.

Signed-off-by: James Almer <[email protected]>
---
 libavformat/iamf_parse.c  | 7 -------
 libavformat/iamf_reader.c | 8 --------
 2 files changed, 15 deletions(-)

diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
index 90bccdb6f9..bc6b69589b 100644
--- a/libavformat/iamf_parse.c
+++ b/libavformat/iamf_parse.c
@@ -633,13 +633,6 @@ static int param_parse(void *s, IAMFContext *c, AVIOContext *pb,
         duration = ffio_read_leb(pb);
         if (!duration)
             return AVERROR_INVALIDDATA;
-        if (audio_element) {
-            const IAMFCodecConfig *codec_config = ff_iamf_get_codec_config(c, audio_element->codec_config_id);
-            if (duration > av_rescale(codec_config->nb_samples, codec_config->sample_rate, parameter_rate)) {
-                av_log(s, AV_LOG_ERROR, "Invalid block duration in parameter_id %u\n", parameter_id);
-                return AVERROR_INVALIDDATA;
-            }
-        }
         constant_subblock_duration = ffio_read_leb(pb);
         if (constant_subblock_duration == 0)
             nb_subblocks = ffio_read_leb(pb);
diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
index 1b963ee311..5ab7ff16b2 100644
--- a/libavformat/iamf_reader.c
+++ b/libavformat/iamf_reader.c
@@ -150,14 +150,6 @@ static int parameter_block_obu(AVFormatContext *s, IAMFDemuxContext *c,
             ret = AVERROR_INVALIDDATA;
             goto fail;
         }
-        if (audio_element) {
-            const IAMFCodecConfig *codec_config = ff_iamf_get_codec_config(&c->iamf, audio_element->codec_config_id);
-            if (duration > av_rescale(codec_config->nb_samples, codec_config->sample_rate, param->parameter_rate)) {
-                av_log(s, AV_LOG_ERROR, "Invalid block duration in parameter_id %u\n", parameter_id);
-                ret = AVERROR_INVALIDDATA;
-                goto fail;
-            }
-        }
         constant_subblock_duration = ffio_read_leb(pb);
         if (constant_subblock_duration == 0)
             nb_subblocks = ffio_read_leb(pb);
-- 
2.52.0


>From d37c21aa8d8e81328dafdbcd372a5059f16ab1b0 Mon Sep 17 00:00:00 2001
From: James Almer <[email protected]>
Date: Sat, 25 Jul 2026 13:01:07 -0300
Subject: [PATCH 3/3] avformat/iamf_parse: check that Parameter Definitions
 subblocks values are valid

Do a dry run of the subblocks structure before allocating an AVIAMFParamDefinition.

Signed-off-by: James Almer <[email protected]>
---
 libavformat/iamf_parse.c | 123 ++++++++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 46 deletions(-)

diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
index bc6b69589b..a12f4c2350 100644
--- a/libavformat/iamf_parse.c
+++ b/libavformat/iamf_parse.c
@@ -606,6 +606,70 @@ static int ambisonics_config(void *s, AVIOContext *pb,
     return 0;
 }
 
+static int param_parse_subblock(void *s, AVIOContext *pb, AVIAMFParamDefinition *param,
+                                unsigned int mode, unsigned int parameter_id,
+                                unsigned int duration, unsigned int constant_subblock_duration,
+                                unsigned int nb_subblocks, unsigned int type,
+                                const IAMFAudioElement *audio_element)
+{
+    unsigned int total_duration = 0;
+
+    for (int i = 0; i < nb_subblocks; i++) {
+        void *subblock = param ? av_iamf_param_definition_get_subblock(param, i) : NULL;
+        unsigned int subblock_duration = constant_subblock_duration;
+
+        if (constant_subblock_duration == 0) {
+            subblock_duration = ffio_read_leb(pb);
+            if (!param && (duration - total_duration > subblock_duration)) {
+                av_log(s, AV_LOG_ERROR, "Invalid subblock durations in parameter_id %u\n", parameter_id);
+                return AVERROR_INVALIDDATA;
+            }
+            total_duration += subblock_duration;
+        } else if (i == nb_subblocks - 1)
+            subblock_duration = duration - i * constant_subblock_duration;
+
+        switch (type) {
+        case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
+            AVIAMFMixGain *mix = subblock;
+            if (mix)
+                mix->subblock_duration = subblock_duration;
+            break;
+        }
+        case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
+            AVIAMFDemixingInfo *demix = subblock;
+            int dmixp_mode = avio_r8(pb) >> 5;
+            if (demix) {
+                demix->subblock_duration = subblock_duration;
+                // DefaultDemixingInfoParameterData
+                demix->dmixp_mode = dmixp_mode;
+            }
+            av_assert0(audio_element);
+            audio_element->element->default_w = avio_r8(pb) >> 4;
+            break;
+        }
+        case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
+            AVIAMFReconGain *recon = subblock;
+            if (recon)
+                recon->subblock_duration = subblock_duration;
+            break;
+        }
+        default:
+            return AVERROR_INVALIDDATA;
+        }
+
+        if (avio_feof(pb))
+            return AVERROR_INVALIDDATA;
+    }
+
+    if (mode == 0 && constant_subblock_duration == 0 && total_duration != duration) {
+        av_log(s, AV_LOG_ERROR, "Invalid subblock durations in parameter_id %u\n", parameter_id);
+        av_free(param);
+        return AVERROR_INVALIDDATA;
+    }
+
+    return 0;
+}
+
 static int param_parse(void *s, IAMFContext *c, AVIOContext *pb,
                        unsigned int type,
                        const IAMFAudioElement *audio_element,
@@ -615,7 +679,6 @@ static int param_parse(void *s, IAMFContext *c, AVIOContext *pb,
     AVIAMFParamDefinition *param;
     unsigned int parameter_id, parameter_rate, mode;
     unsigned int duration = 0, constant_subblock_duration = 0, nb_subblocks = 0;
-    unsigned int total_duration = 0;
     size_t param_size;
 
     parameter_id = ffio_read_leb(pb);
@@ -642,7 +705,6 @@ static int param_parse(void *s, IAMFContext *c, AVIOContext *pb,
                 return AVERROR_INVALIDDATA;
             }
             nb_subblocks = duration / constant_subblock_duration;
-            total_duration = duration;
         }
     }
 
@@ -651,55 +713,24 @@ static int param_parse(void *s, IAMFContext *c, AVIOContext *pb,
         return AVERROR_INVALIDDATA;
     }
 
+    // Do a dry run to ensure the amount of calculated nb_subblocks are in fact coded in the bitstream
+    int64_t pos = avio_tell(pb);
+    int ret = param_parse_subblock(s, pb, NULL, mode, parameter_id, duration, constant_subblock_duration,
+                                   nb_subblocks, type, audio_element);
+    if (ret < 0)
+        return ret;
+
     param = av_iamf_param_definition_alloc(type, nb_subblocks, &param_size);
     if (!param)
         return AVERROR(ENOMEM);
 
-    for (int i = 0; i < nb_subblocks; i++) {
-        void *subblock = av_iamf_param_definition_get_subblock(param, i);
-        unsigned int subblock_duration = constant_subblock_duration;
-
-        if (constant_subblock_duration == 0) {
-            subblock_duration = ffio_read_leb(pb);
-            if (duration - total_duration > subblock_duration) {
-                av_log(s, AV_LOG_ERROR, "Invalid subblock durations in parameter_id %u\n", parameter_id);
-                av_free(param);
-                return AVERROR_INVALIDDATA;
-            }
-            total_duration += subblock_duration;
-        } else if (i == nb_subblocks - 1)
-            subblock_duration = duration - i * constant_subblock_duration;
-
-        switch (type) {
-        case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
-            AVIAMFMixGain *mix = subblock;
-            mix->subblock_duration = subblock_duration;
-            break;
-        }
-        case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
-            AVIAMFDemixingInfo *demix = subblock;
-            demix->subblock_duration = subblock_duration;
-            // DefaultDemixingInfoParameterData
-            av_assert0(audio_element);
-            demix->dmixp_mode = avio_r8(pb) >> 5;
-            audio_element->element->default_w = avio_r8(pb) >> 4;
-            break;
-        }
-        case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
-            AVIAMFReconGain *recon = subblock;
-            recon->subblock_duration = subblock_duration;
-            break;
-        }
-        default:
-            av_free(param);
-            return AVERROR_INVALIDDATA;
-        }
-    }
-
-    if (!mode && !constant_subblock_duration && total_duration != duration) {
-        av_log(s, AV_LOG_ERROR, "Invalid subblock durations in parameter_id %u\n", parameter_id);
+    // Parse the subblocks again and commit them now that we know they are valid.
+    avio_seek(pb, pos, SEEK_SET);
+    ret = param_parse_subblock(s, pb, param, mode, parameter_id, duration, constant_subblock_duration,
+                               nb_subblocks, type, audio_element);
+    if (ret < 0) {
         av_free(param);
-        return AVERROR_INVALIDDATA;
+        return ret;
     }
 
     param->parameter_id = parameter_id;
-- 
2.52.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]