[FFmpeg-devel] [PATCH] Fix signed integer overflows

[Vitaly Buka]

On Fri, Aug 18, 2017 at 1:11 AM, Tomas Härdin <tjoppen at acc.umu.se> wrote:

> On 2017-08-18 08:14, Vitaly Buka wrote:
>
>> Signed integer overflow is undefined behavior.
>> Detected with clang and -fsanitize=signed-integer-overflow
>>
>> Signed-off-by: Vitaly Buka <vitalybuka at google.com>
>> ---
>>   libavcodec/utils.c    | 2 +-
>>   libavformat/aviobuf.c | 4 +++-
>>   libavformat/mov.c     | 2 +-
>>   3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 1336e921c9..024dc1f3e2 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -971,7 +971,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>           }
>>             if (!avctx->rc_initial_buffer_occupancy)
>> -            avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size
>> * 3 / 4;
>> +            avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size
>> * 3ll / 4;
>>
>
> Didn't know you could use lower case for long long constants. Neat
>
>           if (avctx->ticks_per_frame && avctx->time_base.num &&
>>               avctx->ticks_per_frame > INT_MAX / avctx->time_base.num) {
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index 7f4e740a33..319a402faf 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -259,7 +259,9 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int
>> whence)
>>           offset1 = pos + (s->buf_ptr - s->buffer);
>>           if (offset == 0)
>>               return offset1;
>> -        offset += offset1;
>> +        // Use unsigned type to avoid undefined behavior of singed
>> overflow.
>> +        // Code below will report error on overflow anyway.
>> +        offset += (uint64_t)offset1;
>>
>
> I presume offset1 is some value which is never >= 1<<63?

Yes. it's it int64 so max value is (1ll<<63 - 1)

>


>
>
>       }
>>       if (offset < 0)
>>           return AVERROR(EINVAL);
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 522ce60c2d..a14c9f182b 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -5572,7 +5572,7 @@ static int mov_read_default(MOVContext *c,
>> AVIOContext *pb, MOVAtom atom)
>>         if (atom.size < 0)
>>           atom.size = INT64_MAX;
>> -    while (total_size + 8 <= atom.size && !avio_feof(pb)) {
>> +    while (total_size <= atom.size - 8 && !avio_feof(pb)) {
>>
>
> atom.size can never be < 8?
>
It does not matter. We just need atom.size never < (INT64_MIN + 8)



>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>