[FFmpeg-devel] [PATCH 2/2] Do not fail DVB sub decoding because of a few padding bytes

[Måns Rullgård]

Justin Ruggles <justin.ruggles at gmail.com> writes:

> On 02/09/2011 01:32 PM, Reimar D?ffinger wrote:
>
>> Instead of returning an error when bytes are left over, just return
>> the number of actually used bytes as other decoders do.
>> Instead add a special case so an error will be returned when none
>> of the data looks valid to avoid making debugging a pain.
>> ---
>>  libavcodec/dvbsubdec.c |    9 ++-------
>>  1 files changed, 2 insertions(+), 7 deletions(-)
>> 
>> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
>> index 8cc8d4f..401144f 100644
>> --- a/libavcodec/dvbsubdec.c
>> +++ b/libavcodec/dvbsubdec.c
>> @@ -1423,7 +1423,7 @@ static int dvbsub_decode(AVCodecContext *avctx,
>>  
>>  #endif
>>  
>> -    if (buf_size <= 2)
>> +    if (buf_size <= 2 || *buf != 0x0f)
>>          return -1;
>>  
>>      p = buf;
>> @@ -1467,12 +1467,7 @@ static int dvbsub_decode(AVCodecContext *avctx,
>>          p += segment_length;
>>      }
>>  
>> -    if (p != p_end) {
>> -        av_dlog(avctx, "Junk at end of packet\n");
>> -        return -1;
>> -    }
>> -
>> -    return buf_size;
>> +    return p - buf;
>>  }
>
> This looks fine.  Does the decoder still work ok if the first byte of
> the "junk" happens to be 0xF?

Doesn't look like it to me.  If the first byte is 0x0f, it immediately
proceeds to read 6 bytes from the packet, two of which are used
unchecked as a 16-bit size value.  Even without this patch, this code
could easily over-read the buffer if fed with bad data.

-- 
M?ns Rullg?rd
mans at mansr.com