[FFmpeg-devel] ABI break in 4.3

[Marton Balint]

On Sun, 5 Jul 2020, Timo Rothenpieler wrote:

> On 05.07.2020 16:18, Carl Eugen Hoyos wrote:
>> Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt
>> <andreas.rheinhardt at gmail.com>:
>> 
>>> This crash is due to Chromium using av_max_alloc in an undocumented way;
>>> more exactly, the Chromium FFmpeg fork changes the allocation functions
>>> so that if max_alloc_size is zero, the limit is completely disabled; and
>>> of course it also sets the limit to zero. Up until 4.2, this worked with
>>> a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead
>>> max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5
>>> this is no longer so.
>> 
>> I think it is not immediately obvious that this is a (severe!) issue in
>> Chromium which basically disabled a security feature of FFmpeg
>> that was intentionally set to a very conservative (read: not soo
>> secure) value but was completely disabled by somebody who
>> misunderstood the feature (and failed to ask, I mention this
>> because this person's understanding would have implied that we
>> have no clue in C programming whatsoever).
>> 
>> At least one of the "downstream" fixes I saw in the last weeks simply
>> repeat this failure by again removing the security feature instead of
>> removing the wrong call from Chromium.
>> 
>> I am not sure if it really is our responsibility to explain to downstream
>> that valid multimedia files theoretically can allocate arbitrary amounts
>> of memory but that a responsible caller has to limit this amount for
>> nearly every theoretical use case, the more so for browser decoding.
>> 
>> Carl Eugen
>
> Chrome is using a custom allocator, that crashes the entire application 
> on OOM rather than returning NULL.
> So it's not a security issue in their case.

This is based on the assumption that a libav* function only returns NULL 
if there is a memory allocation error. That is simply not true. I am sure 
we can find a function in the codebase which returns NULL because of 
invalid arguments, or some other condition. IMHO they should not make 
this assumption.

Regards,
Marton