[FFmpeg-devel] [PATCH] dvbsubdec: check against buffer overreads

[Måns Rullgård]

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

> On Wed, Feb 09, 2011 at 10:37:16PM +0100, Janne Grunau wrote:
>>
>> I'll send a patch which adds buffer checks.
>
> see below
>
> Janne
>
> --->8---
> Signed-off-by: Janne Grunau <janne-ffmpeg at jannau.net>
> ---
>  libavcodec/dvbsubdec.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> index 401144f..1c6c6f2 100644
> --- a/libavcodec/dvbsubdec.c
> +++ b/libavcodec/dvbsubdec.c
> @@ -1423,13 +1423,15 @@ static int dvbsub_decode(AVCodecContext *avctx,
>
>  #endif
>
> -    if (buf_size <= 2 || *buf != 0x0f)
> +    if (buf_size <= 6 || *buf != 0x0f) {
> +        av_dlog(avctx, "incomplete or broken packet");
>          return -1;
> +    }
>
>      p = buf;
>      p_end = buf + buf_size;
>
> -    while (p < p_end && *p == 0x0f) {
> +    while (p+6 < p_end && *p == 0x0f) {
>          p += 1;
>          segment_type = *p++;
>          page_id = AV_RB16(p);
> @@ -1437,6 +1439,11 @@ static int dvbsub_decode(AVCodecContext *avctx,
>          segment_length = AV_RB16(p);
>          p += 2;
>
> +        if (p+segment_length > p_end) {
> +            av_dlog(avctx, "incomplete packet");
> +            return -1;
> +        }

Not overflow-safe.  If the buffer is near the top of the address space,
adding an oversize segment_length could make it wrap.  More generally,
the C standard makes so much as calculating an invalid address undefined
behaviour.

The correct test for error here is (p_end - p < segment_length).

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