On 9/28/2021 3:03 PM, Michael Niedermayer wrote:
On Tue, Sep 28, 2021 at 09:44:01PM +1000, Peter Ross wrote:
On Tue, Sep 28, 2021 at 12:07:49AM -0300, James Almer wrote:
On 9/27/2021 7:37 PM, Michael Niedermayer wrote:
Fixes: Timeout
Fixes:
39089/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSNSIREN_fuzzer-6677219854909440
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
---
libavcodec/siren.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/siren.c b/libavcodec/siren.c
index 7f2b4678608..708990d6654 100644
--- a/libavcodec/siren.c
+++ b/libavcodec/siren.c
@@ -718,6 +718,9 @@ static int siren_decode(AVCodecContext *avctx, void *data,
if ((ret = init_get_bits8(gb, avpkt->data, avpkt->size)) < 0)
return ret;
+ if (s->sample_rate_bits + 4 > get_bits_left(gb))
+ return AVERROR_INVALIDDATA;
This is not enough. You'll inevitably get another timeout in the future
unless you add a get_bits_left(gb) > 0 condition to the loop in
decode_envelope().
And there should be at least four bits left after decode_envelope() returns,
so check for that too.
I tend not to add more checks than needed for timeouts because
it increases the risk the patch gets rejected because it breaks something and
then the reporter refuses to share any details. Or someone is offended because
too many ugly/hacky timeout checks are added. So my patches are a bit
minimalistic (less work and less friction).
But that now offends others who want more complete fixes :(
added a check in the loop, ill post the new patch
Nothing you did offended me. I'm just saying that your patch fixes the
timeout for that specific bitstream created by the fuzzer, but the core
issue is in that loop, and it'd be better to fix it there instead.
If you don't want to add the check after decode_envelope() then that's ok.
suggest increasing the threshold to include:
s->sample_rate_bits + s->number_of_regions + 4 + s->checksum_bits >
get_bits_left(gb)
changed
hey michael are you only fuzzing AV_CODEC_ID_MSNSIREN?
i would expect this issue to occur for for the vanilla SIREN codec too?
The fuzzer is supposed to fuzz all decoders,
sometimes it has difficulty with some decoders, when for example theres no
seed file or something else special is needed for the codec. For example
a codec may need a specific codec_tag or extradata set ...
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".
