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

[Måns Rullgård]

Janne Grunau <janne-ffmpeg at jannau.net> writes:

> On Wed, Feb 09, 2011 at 09:01:37PM +0000, M?ns Rullg?rd wrote:
>> Janne Grunau <janne-ffmpeg at jannau.net> writes:
>> 
>> > On Wed, Feb 09, 2011 at 07:32:07PM +0100, 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)

What is the point in checking *buf here and again in the loop below?
One of them should be enough.

>> >>          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;
>> >>  }
>> >>  
>> >>  
>> >
>> > still ok, but an answer to my questions would be nice.
>> 
>> The patch increases the chance of garbage triggering a buffer over-read
>> and possibly worse things.
>
> no, it doesn't change behaviour. if the first byte of the garbage is 0x0f
> the decoding loop will continue all the bad things would still happen.

Of course.

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