On 7/3/2022 7:00 AM, Nicolas George wrote:
Andreas Rheinhardt (12022-07-03):
if (!av_bprint_is_complete(bp))
- return AVERROR(ENOMEM);
+ break;
Isn't this actually still against the API? av_channel_layout_describe()
will not return the correct number of bytes necessary to write the
string for the channel layout.
You are both right.
BPrint-based APIs are not supposed to check for truncation, because
printing into a bounded buffer to determine the necessary size is a
valid use (see AV_BPRINT_SIZE_COUNT_ONLY).
What is wrong is Michael's original fix:
commit 8154cb7c2ff2afcb1a0842de8c215b7714c814d0
Author: Michael Niedermayer <mich...@niedermayer.cc>
Date: 2022-06-30 00:00:32 +0200
avutil/channel_layout: av_channel_layout_describe_bprint: Check for buffer
end
Fixes: Timeout printing a billion channels
Fixes:
48099/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6754782204788736
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 21b70173b7..1887987789 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -757,6 +757,10 @@ int av_channel_layout_describe_bprint(const
AVChannelLayout *channel_layout,
if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM &&
channel_layout->u.map[i].name[0])
If the channel count is insanely high, then this will cause invalid
reads.
av_bprintf(bp, "@%s", channel_layout->u.map[i].name);
+
+ if (!av_bprint_is_complete(bp))
+ return AVERROR(ENOMEM);
+
}
if (channel_layout->nb_channels) {
av_bprintf(bp, ")");
Obviously, this fuzzer found a case where a demuxer or a decoder
constructs an invalid channel layout in memory without proper
validation. There is a bug, possibly an security-related one, and this
only hides it from the test suite.
The Matroska demuxer could in theory generate a native layout with more
than 64 channels where popcnt(mask) != channels, and nothing seems to
validate what a demuxer's read_header() callback returns if you just
call lavf API functions like target_dem_fuzzer.c seems to do.
Maybe:
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 73ded761fd..ad7ee390a2 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2950,10 +2950,10 @@ static int matroska_parse_tracks(AVFormatContext *s)
st->codecpar->codec_tag = fourcc;
st->codecpar->sample_rate = track->audio.out_samplerate;
// channel layout may be already set by codec private checks above
- if (st->codecpar->ch_layout.order == AV_CHANNEL_ORDER_NATIVE &&
- !st->codecpar->ch_layout.u.mask)
+ if (!av_channel_layout_check(&st->codecpar->ch_layout)) {
st->codecpar->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
- st->codecpar->ch_layout.nb_channels = track->audio.channels;
+ st->codecpar->ch_layout.nb_channels = track->audio.channels;
+ }
if (!st->codecpar->bits_per_coded_sample)
st->codecpar->bits_per_coded_sample = track->audio.bitdepth;
if (st->codecpar->codec_id == AV_CODEC_ID_MP3 ||
is enough to ensure a valid layout is propagated. This assumes
parameters set by codec private parsing are correct (only FLAC seem to
be present right now, and it is).
Assuming i got this right, in this fuzzing sample's case it would still
have a billion channels (since that's what track->audio.channels
contains, as read from the container), but using the unspec layout,
which is technically valid even if nothing will really handle it, and
av_channel_layout_describe() will print a small string.
Still, i think a check in avformat_open_input() might also be a good
idea, especially once (and if) demuxers start propagating custom
layouts, where the map array will be allocated.
The proper fix is to find where the input is improperly validated, and
then revert this change.
I do not know how to exploit the
"48099/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6754782204788736"
information to reproduce the bug and investigate.
Michael can share the sample, but you need to compile
tools/target_dem_fuzzer.c to read it.
Regards,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
