[FFmpeg-devel] [PATCH v2] avformat/mov: abort reading truncated stts

[Gyan Doshi]

On 2021-12-21 02:24 am, Andreas Rheinhardt wrote:
> Gyan Doshi:
>>
>> On 2021-12-21 02:18 am, Andreas Rheinhardt wrote:
>>> Gyan Doshi:
>>>> Avoids overreading the box and ingesting absurd values into stts_data
>>>> ---
>>>>    libavformat/mov.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>>> index 2aed6e80ef..5a7209837f 100644
>>>> --- a/libavformat/mov.c
>>>> +++ b/libavformat/mov.c
>>>> @@ -2935,6 +2935,11 @@ static int mov_read_stts(MOVContext *c,
>>>> AVIOContext *pb, MOVAtom atom)
>>>>        avio_rb24(pb); /* flags */
>>>>        entries = avio_rb32(pb);
>>>>    +    if (atom.size < 8 + (int64_t)entries*8) {
>>>> +        av_log(c->fc, AV_LOG_ERROR, "Truncated STTS box for st
>>>> %d.\n", c->fc->nb_streams-1);
>>>> +        return AVERROR_INVALIDDATA;
>>>> +    }
>>>> +
>>>>        av_log(c->fc, AV_LOG_TRACE, "track[%u].stts.entries = %u\n",
>>>>                c->fc->nb_streams-1, entries);
>>>>   
>>> This might fix the issue with the fuzzer sample Michael gave you, but
>>> what would stop the fuzzer (or a malicious adversary) from simply using
>>> a gigantic atom size?
>> Do you want the comparison to switch to a strict inequality?
>>
> No, because it might be that the adversary just uses the expected size,
> so this would not fix anything.

There are real world multi-hour files with large stts boxes, so there is 
no robust solution for that, only heuristics.

In any case, handling/recognizing the sample count values that led to a 
prolonged demux in Michael's sample is not germane to this patch.

Regards,
Gyan