[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 11:23:54PM +0000, M?ns Rullg?rd wrote:
>> Janne Grunau <janne-ffmpeg at jannau.net> writes:
>> 
>> > @@ -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).
>
> thanks, fixed
>
> 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..dbeb24a 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) {

This one can also overflow.

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