Re: [FFmpeg-devel] [PATCH 5/5] avformat/mov: Check if heif item name is already allocated

[James Almer <jamrial@gmail.com>]

On 4/26/2024 9:03 PM, Michael Niedermayer wrote:
> On Fri, Apr 26, 2024 at 08:57:02PM -0300, James Almer wrote:
>> On 4/26/2024 8:52 PM, Michael Niedermayer wrote:
>>> Fixes: memleak
>>> Fixes: 68212/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-4963488540721152
>>>
>>> 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 97a24e6737e..5b8278f736e 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -8136,6 +8136,9 @@ static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
>>>        avio_rb24(pb);  // flags.
>>>        size -= 4;
>>> +    if (c->heif_item[idx].name)
>>> +        return AVERROR_INVALIDDATA;
>>
>> I prefer to free the old one before the av_bprint_finalize() call instead of
>> aborting.
> 
> does the format allow this to be changing ?

This code is not supposed to be invoked twice if an iinf box is 
correctly parsed. It can only happen if a second iinf box shows up after 
the first and demuxing wasn't aborted, either because the error was 
ignored, or an infe box within was version < 2.

> 
> because security wise, allowing an attacker to change things is worse
> than allowing her to just set it once.

A second iinf box could show up after the first was not finished 
parsing, and as long as it has no names for the items, it would still 
finish parsing even after this patch. So better just clean old values 
and keep parsing. Further sanity checks will happen when all the items 
are turned into something meant to be exported.

> 
> and heif seems to have a rather high density of issues already, judging
> from how many of the recent mov issues where in heif code
> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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".