On 3/17/2022 9:07 PM, James Almer wrote:
On 3/17/2022 8:52 PM, James Almer wrote:
On 3/17/2022 8:30 PM, Michael Niedermayer wrote:
Fixes: Out of array write
Fixes:
45613/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMALOSSLESS_fuzzer-4539073606320128
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
---
libavcodec/wmalosslessdec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/wmalosslessdec.c b/libavcodec/wmalosslessdec.c
index cd05b22689..1728920729 100644
--- a/libavcodec/wmalosslessdec.c
+++ b/libavcodec/wmalosslessdec.c
@@ -281,6 +281,9 @@ static av_cold int decode_init(AVCodecContext
*avctx)
av_channel_layout_uninit(&avctx->ch_layout);
av_channel_layout_from_mask(&avctx->ch_layout, channel_mask);
+ if (s->num_channels != avctx->ch_layout.nb_channels)
+ return AVERROR_PATCHWELCOME; //are there non fuzzed files
with this or is it an error ?
s->num_channels at this point is set to the channels count the user
set before calling avcodec_open2() (Normally from lavf), but it could
be anything.
If channel_mask is taken from extradata, maybe it should be used to
set s->num_channels instead of aborting because the user set value and
extradata disagreed.
Also, can you reproduce this crash before 3c933af493? s->num_channels
was being set to the user set channel count too, same as now.
Right, before that commit s->num_channels and avctx->channels were
always the same, but avctx->channel_layout was whatever came from
extradata, and its popcnt could be != avctx->channels.
After it, avctx->ch_layout.nb_channels is always the same as
popcnt(avctx->ch_layout.u.mask), which can be different than
s->num_channels.
I think my suggestion above to use the extradata channel mask and
ignoring the user set channel count is the best approach for this.
Like this maybe (channel_mask could in theory be zero, so in that case
the user set value should be used).
diff --git a/libavcodec/wmalosslessdec.c b/libavcodec/wmalosslessdec.c
index cd05b22689..915add1962 100644
--- a/libavcodec/wmalosslessdec.c
+++ b/libavcodec/wmalosslessdec.c
@@ -197,15 +197,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR_PATCHWELCOME;
}
- s->max_frame_size = MAX_FRAMESIZE * avctx->ch_layout.nb_channels;
- s->frame_data = av_mallocz(s->max_frame_size +
AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->frame_data)
- return AVERROR(ENOMEM);
-
- s->avctx = avctx;
- ff_llauddsp_init(&s->dsp);
- init_put_bits(&s->pb, s->frame_data, s->max_frame_size);
-
if (avctx->extradata_size >= 18) {
s->decode_flags = AV_RL16(edata_ptr + 14);
channel_mask = AV_RL32(edata_ptr + 2);
@@ -230,6 +221,33 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR_PATCHWELCOME;
}
+ if (channel_mask) {
+ av_channel_layout_uninit(&avctx->ch_layout);
+ av_channel_layout_from_mask(&avctx->ch_layout, channel_mask);
+ } else
+ avctx->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
+
+ s->num_channels = avctx->ch_layout.nb_channels;
+
+ /* extract lfe channel position */
+ s->lfe_channel = -1;
+
+ if (channel_mask & 8) {
+ unsigned int mask;
+ for (mask = 1; mask < 16; mask <<= 1)
+ if (channel_mask & mask)
+ ++s->lfe_channel;
+ }
+
+ s->max_frame_size = MAX_FRAMESIZE * avctx->ch_layout.nb_channels;
+ s->frame_data = av_mallocz(s->max_frame_size +
AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!s->frame_data)
+ return AVERROR(ENOMEM);
+
+ s->avctx = avctx;
+ ff_llauddsp_init(&s->dsp);
+ init_put_bits(&s->pb, s->frame_data, s->max_frame_size);
+
/* generic init */
s->log2_frame_size = av_log2(avctx->block_align) + 4;
@@ -263,24 +281,10 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR_INVALIDDATA;
}
- s->num_channels = avctx->ch_layout.nb_channels;
-
- /* extract lfe channel position */
- s->lfe_channel = -1;
-
- if (channel_mask & 8) {
- unsigned int mask;
- for (mask = 1; mask < 16; mask <<= 1)
- if (channel_mask & mask)
- ++s->lfe_channel;
- }
-
s->frame = av_frame_alloc();
if (!s->frame)
return AVERROR(ENOMEM);
- av_channel_layout_uninit(&avctx->ch_layout);
- av_channel_layout_from_mask(&avctx->ch_layout, channel_mask);
return 0;
}
_______________________________________________
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".
