[FFmpeg-devel] [PATCH]lavf/mov: Do not blindly allocate stts entries

[Carl Eugen Hoyos]

2017-12-31 22:26 GMT+01:00 Carl Eugen Hoyos <ceffmpeg at gmail.com>:
> 2017-12-31 22:17 GMT+01:00 Michael Niedermayer <michael at niedermayer.cc>:
>> On Sat, Dec 30, 2017 at 02:36:39PM +0100, Carl Eugen Hoyos wrote:
>>> 2017-12-29 23:37 GMT+01:00 Carl Eugen Hoyos <ceffmpeg at gmail.com>:
>>> > 2017-11-28 21:32 GMT+01:00 Michael Niedermayer <michael at niedermayer.cc>:
>>> >> On Mon, Nov 27, 2017 at 05:24:14AM +0100, Carl Eugen Hoyos wrote:
>>> >
>>> >>>      for (i = 0; i < entries && !pb->eof_reached; i++) {
>>> >>> -        int sample_duration;
>>> >>> +        int sample_duration, ret;
>>> >>>          unsigned int sample_count;
>>> >>> +        if (i > sc->stts_count) {
>>> >>> +            ret = av_reallocp_array(&sc->stts_data,
>>> >>> +                                    FFMIN(sc->stts_count * 2LL, entries),
>>> >>> +                                    sizeof(*sc->stts_data));
>>> >>
>>> >> this should use a variant of av_fast_realloc
>>> >
>>> > Do you prefer the new patch?
>>> > The old variant here looks slightly saner to me.
>>>
>>> Attached is what you possibly had in mind.
>>>
>>> Please review, Carl Eugen
>>
>>>  mov.c |   13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>> cc7986179fe0ddc394457e8543d9ae907b49373c  0001-lavf-mov-Use-av_fast_realloc-in-mov_read_stts.patch
>>> From f5fcd9ed1e5ce604c358a3787f1977277005ebb5 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
>>> Date: Sat, 30 Dec 2017 14:34:41 +0100
>>> Subject: [PATCH] lavf/mov: Use av_fast_realloc() in mov_read_stts().
>>>
>>> Avoids large allocations for short files with invalid stts entry.
>>> Fixes bugzilla 1102.
>>> ---
>>>  libavformat/mov.c |   13 +++++++++++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index 2064473..1e97652 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -2850,13 +2850,22 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>>          av_log(c->fc, AV_LOG_WARNING, "Duplicated STTS atom\n");
>>>      av_free(sc->stts_data);
>>>      sc->stts_count = 0;
>>> -    sc->stts_data = av_malloc_array(entries, sizeof(*sc->stts_data));
>>> -    if (!sc->stts_data)
>>> +    if (entries >= INT_MAX / sizeof(*sc->stts_data))
>>>          return AVERROR(ENOMEM);
>>
>> this leaves a stale pointer on error in sc->stts_data
>
> New patch attached.

Which would not have worked as intended, new variant attached.

Carl Eugen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-mov-Use-av_fast_realloc-in-mov_read_stts.patch
Type: text/x-patch
Size: 1945 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171231/1d139edf/attachment.bin>