* [FFmpeg-devel] [PATCH] iamf (PR #21115)
@ 2025-12-06 1:56 James Almer via ffmpeg-devel
0 siblings, 0 replies; only message in thread
From: James Almer via ffmpeg-devel @ 2025-12-06 1:56 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: James Almer
PR #21115 opened by James Almer (jamrial)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21115
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21115.patch
An ASAN heap-buffer-overflow in scalable_channel_layout_config was caused by an unchecked assumption that the channel layout of a scalable audio layer is a superset of the previous layer's channel layout.
`scalable_channel_layout_config` constructs a channel layout map by copying channels from the previous layer and adding new ones. The memory allocation is based on the target loudspeaker_layout. However, if the target layout doesn't encompass all previous channels (e.g., Mono to Stereo), copying previous channels followed by adding current ones could exceed the allocated size, causing a heap buffer overflow.
This PR adds an exception for the know case of Mono -> Stereo, and a check to ensure the previous layer's channel layout is a subset of the current layer's layout by comparing their masks. If the condition isn't met, an error is returned.
This supersedes #21107
>From ea3d8ad34d47dbf18ed0c4af47162263e1eb1db1 Mon Sep 17 00:00:00 2001
From: James Almer <jamrial@gmail.com>
Date: Fri, 5 Dec 2025 22:40:40 -0300
Subject: [PATCH 1/2] avformat/iamf_parse: fix parsing of Scalable layouts with
Mono and Stereo layers
And ASAN heap-buffer-overflow in scalable_channel_layout_config was caused by
an unchecked assumption that the channel layout of a scalable audio layer is a
superset of the previous layer's channel layout.
scalable_channel_layout_config constructs a channel layout map by copying
channels from the previous layer and adding new ones. The memory allocation is
based on the target loudspeaker_layout. However, if the target layout doesn't
encompass all previous channels (e.g., Mono to Stereo), copying previous
channels followed by adding current ones could exceed the allocated size,
causing a heap buffer overflow.
This commit adds an exception for the know case of Mono -> Stereo, and a check
to ensure the previous layer's channel layout is a subset of the current
layer's layout by comparing their masks. If the condition isn't met,
an error is returned.
Co-authored-by: Oliver Chang <ochang@google.com>
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavformat/iamf_parse.c | 79 ++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 32 deletions(-)
diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
index 597d800be0..3d78533faf 100644
--- a/libavformat/iamf_parse.c
+++ b/libavformat/iamf_parse.c
@@ -347,6 +347,41 @@ static int update_extradata(AVCodecParameters *codecpar)
return 0;
}
+static int parse_coupled_substream(AVChannelLayout *out, AVChannelLayout *in, int n)
+{
+ if (in->u.mask & AV_CH_LAYOUT_STEREO) {
+ out->u.map[n++].id = AV_CHAN_FRONT_LEFT;
+ out->u.map[n++].id = AV_CHAN_FRONT_RIGHT;
+ in->u.mask &= ~AV_CH_LAYOUT_STEREO;
+ } else if (in->u.mask & (AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER)) {
+ out->u.map[n++].id = AV_CHAN_FRONT_LEFT_OF_CENTER;
+ out->u.map[n++].id = AV_CHAN_FRONT_RIGHT_OF_CENTER;
+ in->u.mask &= ~(AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER);
+ } else if (in->u.mask & (AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT)) {
+ out->u.map[n++].id = AV_CHAN_SIDE_LEFT;
+ out->u.map[n++].id = AV_CHAN_SIDE_RIGHT;
+ in->u.mask &= ~(AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT);
+ } else if (in->u.mask & (AV_CH_BACK_LEFT|AV_CH_BACK_RIGHT)) {
+ out->u.map[n++].id = AV_CHAN_BACK_LEFT;
+ out->u.map[n++].id = AV_CHAN_BACK_RIGHT;
+ in->u.mask &= ~(AV_CH_BACK_LEFT|AV_CH_BACK_RIGHT);
+ } else if (in->u.mask & (AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT)) {
+ out->u.map[n++].id = AV_CHAN_TOP_FRONT_LEFT;
+ out->u.map[n++].id = AV_CHAN_TOP_FRONT_RIGHT;
+ in->u.mask &= ~(AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT);
+ } else if (in->u.mask & (AV_CH_TOP_SIDE_LEFT|AV_CH_TOP_SIDE_RIGHT)) {
+ out->u.map[n++].id = AV_CHAN_TOP_SIDE_LEFT;
+ out->u.map[n++].id = AV_CHAN_TOP_SIDE_RIGHT;
+ in->u.mask &= ~(AV_CH_TOP_SIDE_LEFT|AV_CH_TOP_SIDE_RIGHT);
+ } else if (in->u.mask & (AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT)) {
+ out->u.map[n++].id = AV_CHAN_TOP_BACK_LEFT;
+ out->u.map[n++].id = AV_CHAN_TOP_BACK_RIGHT;
+ in->u.mask &= ~(AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT);
+ }
+
+ return n;
+}
+
static int scalable_channel_layout_config(void *s, AVIOContext *pb,
IAMFAudioElement *audio_element,
const IAMFCodecConfig *codec_config)
@@ -395,12 +430,19 @@ static int scalable_channel_layout_config(void *s, AVIOContext *pb,
if (!i && loudspeaker_layout == 15)
expanded_loudspeaker_layout = avio_r8(pb);
- if (expanded_loudspeaker_layout > 0 && expanded_loudspeaker_layout < 13) {
+ if (expanded_loudspeaker_layout >= 0 && expanded_loudspeaker_layout < 13) {
av_channel_layout_copy(&ch_layout, &ff_iamf_expanded_scalable_ch_layouts[expanded_loudspeaker_layout]);
} else if (loudspeaker_layout < 10) {
av_channel_layout_copy(&ch_layout, &ff_iamf_scalable_ch_layouts[loudspeaker_layout]);
- if (i)
- ch_layout.u.mask &= ~av_channel_layout_subset(&audio_element->element->layers[i-1]->ch_layout, UINT64_MAX);
+ if (i) {
+ uint64_t mask = av_channel_layout_subset(&audio_element->element->layers[i-1]->ch_layout, UINT64_MAX);
+ // When the first layer is Mono, the second layer may not have the C channel (e.g. Stereo)
+ if (audio_element->element->layers[i-1]->ch_layout.nb_channels == 1)
+ n--;
+ else if ((ch_layout.u.mask & mask) != mask)
+ return AVERROR_INVALIDDATA;
+ ch_layout.u.mask &= ~mask;
+ }
} else
ch_layout = (AVChannelLayout){ .order = AV_CHANNEL_ORDER_UNSPEC,
.nb_channels = substream_count +
@@ -430,38 +472,11 @@ static int scalable_channel_layout_config(void *s, AVIOContext *pb,
coupled_substream_count = audio_element->layers[i].coupled_substream_count;
while (coupled_substream_count--) {
- if (ch_layout.u.mask & AV_CH_LAYOUT_STEREO) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_FRONT_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_FRONT_RIGHT;
- ch_layout.u.mask &= ~AV_CH_LAYOUT_STEREO;
- } else if (ch_layout.u.mask & (AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_FRONT_LEFT_OF_CENTER;
- layer->ch_layout.u.map[n++].id = AV_CHAN_FRONT_RIGHT_OF_CENTER;
- ch_layout.u.mask &= ~(AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER);
- } else if (ch_layout.u.mask & (AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_SIDE_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_SIDE_RIGHT;
- ch_layout.u.mask &= ~(AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT);
- } else if (ch_layout.u.mask & (AV_CH_BACK_LEFT|AV_CH_BACK_RIGHT)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_BACK_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_BACK_RIGHT;
- ch_layout.u.mask &= ~(AV_CH_BACK_LEFT|AV_CH_BACK_RIGHT);
- } else if (ch_layout.u.mask & (AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_FRONT_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_FRONT_RIGHT;
- ch_layout.u.mask &= ~(AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT);
- } else if (ch_layout.u.mask & (AV_CH_TOP_SIDE_LEFT|AV_CH_TOP_SIDE_RIGHT)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_SIDE_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_SIDE_RIGHT;
- ch_layout.u.mask &= ~(AV_CH_TOP_SIDE_LEFT|AV_CH_TOP_SIDE_RIGHT);
- } else if (ch_layout.u.mask & (AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT)) {
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_BACK_LEFT;
- layer->ch_layout.u.map[n++].id = AV_CHAN_TOP_BACK_RIGHT;
- ch_layout.u.mask &= ~(AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_RIGHT);
- }
+ n = parse_coupled_substream(&layer->ch_layout, &ch_layout, n);
}
substream_count -= audio_element->layers[i].coupled_substream_count;
+ n = parse_coupled_substream(&layer->ch_layout, &ch_layout, n); // In case the first layer is Mono
while (substream_count--) {
if (ch_layout.u.mask & AV_CH_FRONT_CENTER) {
layer->ch_layout.u.map[n++].id = AV_CHAN_FRONT_CENTER;
--
2.49.1
>From b319de8b62e475ca493ddf302ea995ec5e7535ad Mon Sep 17 00:00:00 2001
From: James Almer <jamrial@gmail.com>
Date: Fri, 5 Dec 2025 22:53:22 -0300
Subject: [PATCH 2/2] avformat/iamf_parse: add a few extra sanity checks
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavformat/iamf_parse.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
index 3d78533faf..756d9f0f09 100644
--- a/libavformat/iamf_parse.c
+++ b/libavformat/iamf_parse.c
@@ -418,7 +418,8 @@ static int scalable_channel_layout_config(void *s, AVIOContext *pb,
substream_count = avio_r8(pb);
coupled_substream_count = avio_r8(pb);
- if (substream_count + k > audio_element->nb_substreams)
+ if (!substream_count || coupled_substream_count > substream_count ||
+ substream_count + k > audio_element->nb_substreams)
return AVERROR_INVALIDDATA;
audio_element->layers[i].substream_count = substream_count;
@@ -488,6 +489,9 @@ static int scalable_channel_layout_config(void *s, AVIOContext *pb,
}
}
+ if (n != ch_layout.nb_channels)
+ return AVERROR_INVALIDDATA;
+
ret = av_channel_layout_retype(&layer->ch_layout, AV_CHANNEL_ORDER_NATIVE, 0);
if (ret < 0 && ret != AVERROR(ENOSYS))
return ret;
--
2.49.1
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2025-12-06 1:57 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-06 1:56 [FFmpeg-devel] [PATCH] iamf (PR #21115) James Almer via ffmpeg-devel
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git