[FFmpeg-devel] [PATCH 1/3] avformat/cafdec: sanity check channels and bps

[James Almer]

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 <michael at 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.

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.

> +        return AVERROR_INVALIDDATA;
> +
>       if (caf->bytes_per_packet < 0 || caf->frames_per_packet < 0 || st->codecpar->ch_layout.nb_channels < 0)
>           return AVERROR_INVALIDDATA;
>