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

[James Almer]

On 4/1/2024 8:49 PM, Michael Niedermayer wrote:
> 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 at 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)

Applied and backported.