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

[Reimar Döffinger]

On Wed, Feb 09, 2011 at 07:02:43PM +0000, M?ns Rullg?rd wrote:
> 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.

The only thing that changes is the return value, nothing else.
Instead of the first part of the patch I could have changed the last one
to
return p == buf ? -1 : p - buf;
but I considered the method in the patch to be better to understand in the long term.