On 7/4/2022 1:55 PM, Michael Niedermayer wrote:
On Sun, Jul 03, 2022 at 09:27:31PM -0300, James Almer wrote:
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 <[email protected]>
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 <[email protected]>
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.
seems this is fixing it
thanks
Ok, will apply it and revert 8154cb7c2f.
Still, i think a check in avformat_open_input() might also be a good idea,
yes, i agree
especially once (and if) demuxers start propagating custom layouts, where
the map array will be allocated.
[...]
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
