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

[Janne Grunau]

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)
> >>          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.
I'll send a patch which adds buffer checks.

Janne