Re: [FFmpeg-devel] [PATCH 2/6] avformat/mov: Check that tile_item_list is initialized in read_image_iovl()

[Michael Niedermayer <michael@niedermayer.cc>]

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

On Mon, Apr 01, 2024 at 06:54:35PM -0300, James Almer wrote:
> On 4/1/2024 5:56 PM, Michael Niedermayer wrote:
> > Fixes: null pointer dereference
> > Fixes: 67494/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6528714521247744
> > 
> > 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 | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 7bdeeb99f98..fa4c237c0d8 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -9364,6 +9364,10 @@ static int read_image_iovl(AVFormatContext *s, const HEIFGrid *grid,
> >       }
> >       for (int i = 0; i < tile_grid->nb_tiles; i++) {
> > +        if (!grid->tile_item_list[i]) {
> > +            ret = AVERROR_INVALIDDATA;
> > +            goto fail;
> > +        }
> 
> This should not happen. We shouldn't get this far if the array was not
> filled.
> 
> Can you please test the following?
> 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 7bdeeb99f9..fb0113b149 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -9397,8 +9397,9 @@ static int mov_parse_tiles(AVFormatContext *s)
> > 
> >          for (int j = 0; j < grid->nb_tiles; j++) {
> >              int tile_id = grid->tile_id_list[j];
> > +            int k;
> > 
> > -            for (int k = 0; k < mov->nb_heif_item; k++) {
> > +            for (k = 0; k < mov->nb_heif_item; k++) {
> >                  HEIFItem *item = &mov->heif_item[k];
> >                  AVStream *st = item->st;
> > 
> > @@ -9424,6 +9425,13 @@ static int mov_parse_tiles(AVFormatContext *s)
> >                  break;
> >              }
> > 
> > +            if (k == grid->nb_tiles) {
> > +                av_log(s, AV_LOG_WARNING, "HEIF item id %d referenced by grid id %d doesn't "
> > +                                          "exist\n",
> > +                       tile_id, grid->item->item_id);
> > +                ff_remove_stream_group(s, stg);
> > +                loop = 0;
> > +            }
> >              if (!loop)
> >                  break;
> >          }

i confirm the code fixes the issue, please apply (if it passes fate) and backport (if needed)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.

[-- 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".