On 6/29/2024 8:37 PM, Michael Niedermayer wrote:
On Wed, Jun 26, 2024 at 09:52:44PM -0300, James Almer wrote:
On 3/22/2024 8:08 PM, Michael Niedermayer wrote:
Fixes: Timeout
Fixes:
67044/clusterfuzz-testcase-minimized-ffmpeg_dem_CAF_fuzzer-5791144363491328
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
---
libavformat/cafdec.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libavformat/cafdec.c b/libavformat/cafdec.c
index 426c56b9bd..334077efb5 100644
--- a/libavformat/cafdec.c
+++ b/libavformat/cafdec.c
@@ -33,6 +33,7 @@
#include "isom.h"
#include "mov_chan.h"
#include "libavcodec/flac.h"
+#include "libavcodec/internal.h"
#include "libavutil/intreadwrite.h"
#include "libavutil/intfloat.h"
#include "libavutil/dict.h"
@@ -87,6 +88,10 @@ static int read_desc_chunk(AVFormatContext *s)
st->codecpar->ch_layout.nb_channels = avio_rb32(pb);
st->codecpar->bits_per_coded_sample = avio_rb32(pb);
+ if (st->codecpar->ch_layout.nb_channels > FF_SANE_NB_CHANNELS ||
+ st->codecpar->bits_per_coded_sample > 64)
Where does the process take so long that oss-fuzz gets a timeout if these
are unreasonably high? I don't see nb_channels used anywhere in here where
that matters.
Is it in the PCM decoder? Because that decoder is meant to handle any
arbitrary amount of channels, so limiting it to whatever FF_SANE_NB_CHANNELS
is set to is not ok.
libavutil/channel_layout.c:627:17
or it will OOM before depending on how much memory is available
Does this fix the timeout?
diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 2d6963b6df..a4fcd199e1 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -623,6 +623,8 @@ int av_channel_layout_describe_bprint(const AVChannelLayout
*channel_layout,
for (i = 0; i < channel_layout->nb_channels; i++) {
enum AVChannel ch =
av_channel_layout_channel_from_index(channel_layout, i);
+ if (!av_bprint_is_complete(bp))
+ break;
if (i)
av_bprintf(bp, "+");
av_channel_name_bprint(bp, ch);
But this is not ok as it will affect the buffer len value
av_channel_layout_describe() returns on success when truncation took
place, so something else would have to be done.
And is the bits_per_coded_sample > 64 check to prevent codec_id being
AV_CODEC_ID_NONE? if so, how does that affect demuxing time?
AV_CODEC_ID_NONE for that matter could happen for valid files with a codec
we don't currently support.
We generally check read values for validity at the earliest,
bits_per_coded_sample of more than 64 seem not valid to me.
I agree the check is fine, but my point is that, assuming this is
affecting demuxing time, this check as is may be hiding an issue related
to codec_id being set to AV_CODEC_ID_NONE here (the result of
ff_get_pcm_codec_id() with an unsupported bits_per_coded_sample value),
so it should be looked at more closely because said codec_id could
happen with valid files using codecs the demuxer does not know about.
If it does not affect demuxing time and is a "just in case" check, then
it doesn't belong in a patch that says "Fixes: Timeout" and mentions an
ossfuzz issue.
thx
[...]
_______________________________________________
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".
