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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 4398 bytes --]

On Thu, Jun 19, 2025 at 09:53:33PM -0300, James Almer wrote:
> 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.

sample send privately to you.

The code above does not fix it (had to apply by hand though it didnt apply

unning: 416811958/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5425269114732544
libavformat/mov.c:10342:9: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavformat/mov.c:10342:9 in
libavformat/mov.c:10342:9: runtime error: store to null pointer of type 'unsigned int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavformat/mov.c:10342:9 in
AddressSanitizer:DEADLYSIGNAL
=================================================================
==305816==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000b1766e bp 0x7ffe03383c90 sp 0x7ffe03383960 T0)
==305816==The signal is caused by a WRITE memory access.
==305816==Hint: address points to the zero page.
    #0 0xb1766e in mov_parse_heif_items ffmpeg/libavformat/mov.c:10342:30
    #1 0xb1766e in mov_read_header ffmpeg/libavformat/mov.c:10498:15
    #2 0x79457d in avformat_open_input ffmpeg/libavformat/demux.c:309:20
    #3 0x5b1fd2 in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:199:11
    #4 0x2729e4c in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (ffmpeg/tools/target_dem_mov_fuzzer+0x2729e4c)
    #5 0x27144bf in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (ffmpeg/tools/target_dem_mov_fuzzer+0x27144bf)
    #6 0x2719b1f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (ffmpeg/tools/target_dem_mov_fuzzer+0x2719b1f)
    #7 0x271415b in main (ffmpeg/tools/target_dem_mov_fuzzer+0x271415b)
    #8 0x7fdaeca5b082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #9 0x504f5d in _start (ffmpeg/tools/target_dem_mov_fuzzer+0x504f5d)


thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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".