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

[Anatoly Nenashev]

On 26.01.2011 08:51, Alex Converse wrote:
> 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
>
>    

Thanks for review! I also have a hope to see Michael's comments about it.