[FFmpeg-devel] [PATCH] Fix decoding crash on some trashed interlaced MPEG2 streams. This fixes issue 2367.

[Anatoly Nenashev]

On 18.02.2011 15:26, M?ns Rullg?rd wrote:
> Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>  writes:
>
>    
>> On 18.02.2011 03:47, M?ns Rullg?rd wrote:
>>      
>>> Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>   writes:
>>>
>>>
>>>        
>>>>    From 20e1e656ab07edbfaec5e329a408139720c6b8e7 Mon Sep 17 00:00:00 2001
>>>> From: anatoly<anatoly.nenashev at ovsoft.ru>
>>>> Date: Fri, 18 Feb 2011 01:01:45 +0300
>>>> Subject: [PATCH] Fix decoding crash on some trashed interlaced MPEG2 streams. This fixes issue 2367.
>>>>
>>>> ---
>>>>    libavcodec/mpeg12.c |    5 +++++
>>>>    1 files changed, 5 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
>>>> index 3c73627..28292ab 100644
>>>> --- a/libavcodec/mpeg12.c
>>>> +++ b/libavcodec/mpeg12.c
>>>> @@ -1763,7 +1763,12 @@ static int mpeg_decode_slice(Mpeg1Context *s1, int mb_y,
>>>>            s->dest[1] +=(16>>   lowres)>>   s->chroma_x_shift;
>>>>            s->dest[2] +=(16>>   lowres)>>   s->chroma_x_shift;
>>>>
>>>> +        if (s->mb_intra || !field_pic || s->last_picture.data[0] ||
>>>> +            s->first_field || s->picture_structure != s->field_select[0][0] + 1) {
>>>>
>>>>          
>>> Is it really necessary to do all these checks per MB?
>>>
>>>
>>>        
>> You are right. Checks for "field_pic", "s->last_picture.data[0]",
>> "s->first_field" can be done outside the loop. Changed patch in
>> attachment.
>>
>>
>>  From ecdb1bfe09823bf423a86e6866b129e5bcabf14f Mon Sep 17 00:00:00 2001
>> From: anatoly<anatoly.nenashev at ovsoft.ru>
>> Date: Fri, 18 Feb 2011 12:08:05 +0300
>> Subject: [PATCH] Fix crash of decoder on MPEG2 interlaced streams (issue 2367)
>>
>> ---
>>   libavcodec/mpeg12.c |    7 +++++++
>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
>> index 3c73627..45c1881 100644
>> --- a/libavcodec/mpeg12.c
>> +++ b/libavcodec/mpeg12.c
>> @@ -1640,6 +1640,8 @@ static int mpeg_decode_slice(Mpeg1Context *s1, int mb_y,
>>       AVCodecContext *avctx= s->avctx;
>>       const int field_pic= s->picture_structure != PICT_FRAME;
>>       const int lowres= s->avctx->lowres;
>> +    const int check_mb_inter = field_pic&&  !s->first_field&&
>> +        s->pict_type == FF_P_TYPE&&  !s->last_picture.data[0];
>>
>>       s->resync_mb_x=
>>       s->resync_mb_y= -1;
>> @@ -1763,7 +1765,12 @@ static int mpeg_decode_slice(Mpeg1Context *s1, int mb_y,
>>           s->dest[1] +=(16>>  lowres)>>  s->chroma_x_shift;
>>           s->dest[2] +=(16>>  lowres)>>  s->chroma_x_shift;
>>
>> +        if (!check_mb_inter || s->mb_intra ||
>> +            s->picture_structure != s->field_select[0][0] + 1) {
>>           MPV_decode_mb(s, s->block);
>> +        } else {
>> +            av_log(avctx, AV_LOG_ERROR, "Some restrictions for inter prediction are broken, skipping macroblock\n");
>> +        }
>>
>>      
> What is the actual problem you are trying to detect?  Missing reference
> picture?
>
>    

The problem is available when second field of first decoded interlaced 
picture has P-type. In this case inter prediction can be done from the 
first field of current picture (works fine) or from the second field of 
previous  picture (crashes decoder). Sample exploit attached to issue 
2367. This sample was specially prepared to show the problem.