[FFmpeg-devel] [PATCH 2/4] avformat/iamf_reader: return REDO on failure to read

[James Almer]

On 3/20/2024 10:15 PM, Michael Niedermayer wrote:
> Fixes: null pointer derference
> Fixes: 67007/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6522819204677632
> 
> 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/iamf_reader.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> index 42d20f1ae6..a06aa98cdb 100644
> --- a/libavformat/iamf_reader.c
> +++ b/libavformat/iamf_reader.c
> @@ -26,6 +26,7 @@
>   #include "libavcodec/packet.h"
>   #include "avformat.h"
>   #include "avio_internal.h"
> +#include "demux.h"
>   #include "iamf.h"
>   #include "iamf_parse.h"
>   #include "iamf_reader.h"
> @@ -322,7 +323,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
>               break;
>       }
>   
> -    return read;
> +    return FFERROR_REDO;

Where is the null pointer dereference happening? I don't particularly 
like this approach because ff_iamf_read_packet() is also called by the 
mov demuxer.

>   }
>   
>   void ff_iamf_read_deinit(IAMFDemuxContext *c)

Does the following also help?

> diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> index 42d20f1ae6..4e79691a03 100644
> --- a/libavformat/iamf_reader.c
> +++ b/libavformat/iamf_reader.c
> @@ -311,8 +311,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
>          } else {
>              int64_t offset = avio_skip(pb, obu_size);
>              if (offset < 0) {
> -                ret = offset;
> -                break;
> +                return offset;
>              }
>          }
>          max_size -= len;

Setting ret there and breaking the loop was wrong, as the scope of ret 
doesn't reach outside loop.