[FFmpeg-devel] [PATCH 3/3] mov: prevent overflow during bit rate calculation

[Andreas Cadhalpun]

On 14.12.2016 04:24, Michael Niedermayer wrote:
> On Wed, Dec 14, 2016 at 01:58:35AM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavformat/mov.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 6c8affc..fc0b25c 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -5887,8 +5887,15 @@ static int mov_read_header(AVFormatContext *s)
>>          for (i = 0; i < s->nb_streams; i++) {
>>              AVStream *st = s->streams[i];
>>              MOVStreamContext *sc = st->priv_data;
>> -            if (st->duration > 0)
>> +            if (st->duration > 0) {
>> +                if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
>> +                    av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
>> +                           sc->data_size, sc->time_scale);
>> +                    mov_read_close(s);
>> +                    return AVERROR_INVALIDDATA;
>> +                }
>>                  st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale / st->duration;
>> +            }
>>          }
>>      }
>>  
>> @@ -5897,6 +5904,12 @@ static int mov_read_header(AVFormatContext *s)
>>              AVStream *st = s->streams[i];
>>              MOVStreamContext *sc = st->priv_data;
>>              if (sc->duration_for_fps > 0) {
>> +                if (sc->data_size > INT64_MAX / sc->time_scale / 8) {
>> +                    av_log(s, AV_LOG_ERROR, "Overflow during bit rate calculation %"PRId64" * 8 * %d\n",
>> +                           sc->data_size, sc->time_scale);
>> +                    mov_read_close(s);
>> +                    return AVERROR_INVALIDDATA;
>> +                }
>>                  st->codecpar->bit_rate = sc->data_size * 8 * sc->time_scale /
>>                      sc->duration_for_fps;
> 
> maybe this can be factored somehow

This is just twice, so factoring out isn't really worth it.
(The really repetitive stuff of validating codec parameters will
be factored out properly, but that's still work in progress.)

> but either way probably ok

Pushed.

Best regards,
Andreas