Re: [FFmpeg-devel] [PATCH 5/8] avformat/mov: Check that sample_sizes is allocated in mov_parse_heif_items()

[James Almer <jamrial-at-gmail.com@ffmpeg.org>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2142 bytes --]

On 6/19/2025 9:32 PM, Michael Niedermayer wrote:
> Fixes: NULL pointer dereference
> Fixes: 416811958/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5425269114732544
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/mov.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8a094b1ea0a..22488b517cb 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -10332,6 +10332,9 @@ static int mov_parse_heif_items(AVFormatContext *s)
>   
>           st = item->st;
>           sc = st->priv_data;
> +        if (!sc->sample_sizes)
> +            return AVERROR_INVALIDDATA;
> +
>           st->codecpar->width  = item->width;
>           st->codecpar->height = item->height;

Does the following fix it too?

> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8a094b1ea0..a2a9c10f20 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5430,18 +5430,18 @@ static int heif_add_stream(MOVContext *c, HEIFItem *item)
>      sc->stsc_data[0].first = 1;
>      sc->stsc_data[0].count = 1;
>      sc->stsc_data[0].id = 1;
> -    sc->chunk_count = 1;
>      sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
>      if (!sc->chunk_offsets)
>          return AVERROR(ENOMEM);
> -    sc->sample_count = 1;
> +    sc->chunk_count = 1;
>      sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
>      if (!sc->sample_sizes)
>          return AVERROR(ENOMEM);
> -    sc->stts_count = 1;
> +    sc->sample_count = 1;
>      sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
>      if (!sc->stts_data)
>          return AVERROR(ENOMEM);
> +    sc->stts_count = 1;
>      sc->stts_data[0].count = 1;
>      // Not used for still images. But needed by mov_build_index.
>      sc->stts_data[0].duration = 0;

I'd rather have the checks in sanity_checks() detect this, so if 
sc->sample_sizes is NULL then sc->sample_count should be 0.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".