[FFmpeg-devel] [PATCH 2/3] avformat/mov: Fix parsing of saio/siaz atoms in encrypted content.

Carl Eugen Hoyos ceffmpeg at gmail.com
Sat Jan 6 00:01:05 EET 2018


2018-01-05 22:29 GMT+01:00 Jacob Trimble <modmaker-at-google.com at ffmpeg.org>:
> On Fri, Jan 5, 2018 at 12:41 PM, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
>> 2018-01-05 20:49 GMT+01:00 Jacob Trimble <modmaker-at-google.com at ffmpeg.org>:
>>
>>> +    entry_count = avio_rb32(pb);
>>> +    encryption_index->auxiliary_offsets = av_malloc_array(sizeof(size_t), entry_count);
>>
>> (sizeof(variable) instead of sizeof(type), please.)
>>
>> But since this could be used for a dos attack, please change this
>> to something similar to 1112ba01.
>> If it is easy to avoid it, very short files should not allocate
>> gigabytes.
>
> Switched to calculating the size based on the number of remaining
> bytes and returning an error if it doesn't match what is read.

Sorry if I miss something:

> +    entry_count = (atom.size - 8 - (has_saio_type ? 8 : 0)) / (version == 0 ? 4 : 8);
> +    if (avio_rb32(pb) != entry_count) {
> +        av_log(c->fc, AV_LOG_ERROR, "incorrect entry_count in saio\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    encryption_index->auxiliary_offsets =
> +        av_malloc_array(sizeof(*encryption_index->auxiliary_offsets), entry_count);

Does this avoid a 1G allocation for a file of a few bytes?

Didn't you simply increase the number of needed bytes to change in a valid
mov file to behave maliciously from one to two?

Thank you, Carl Eugen


More information about the ffmpeg-devel mailing list