[FFmpeg-devel] [PATCH 1/2] MxPEG decoder

[Alex Converse]

On Tue, Jan 25, 2011 at 9:38 AM, Anatoly Nenashev
<anatoly.nenashev at ovsoft.ru> wrote:
> On 29.11.2010 20:51, Anatoly Nenashev wrote:
>>
>> On 29.11.2010 16:12, Anatoly Nenashev wrote:
>>>
>>> On 25.11.2010 18:26, Michael Niedermayer wrote:
>>>>
>>>> On Mon, Nov 08, 2010 at 01:40:39PM +0300, Anatoly Nenashev wrote:
>>>>>
>>>>> [...]
>>>>> I think I've found a solution for this issue. If input packet doesn't
>>>>> contain SOF data then the new picture is allocated from
>>>>> reference_picture which is initiated at decode_frame end. Thus
>>>>> reference_picture is always good. For more details see attachment.
>>>>
>>>> the issue i described has not been fixed
>>>> a invalid SOF still can lead to inconsistant values and your code still
>>>> naively
>>>> sets got_picture=1 indicating a valid SOF even if that is not so.
>>>>
>>>> Fundamentally i think the problem is that you write the code while
>>>> ignoring
>>>> security aspects entirely and expect review to find security issues.
>>>> You should make sure your code is secure and no crafted input no matter
>>>> how
>>>> evil and malformed can lead to any crash or exploit before you submit
>>>> your
>>>> code.
>>>>
>>>>
>>>> [...]
>>>>
>>>
>>> I've reimplemented decoder to be more secure. There is additional flag
>>> named "got_sof_data" which shows that SOF data is succesfully parsed.
>>> Also ugly picture reallocation removed.
>>>
>> Add dimensions check for current and reference picture.
>> Patch tested under valgrind and on trashed stream.
>
> Ping.

This looks pretty good to me.

The only issues I see are minor cosmetics that can be resolved later.

Regards,
Alex Converse