[FFmpeg-devel] [PATCH] add sanity checks to truemotion2 (issue 2512)

[Stefano Sabatini]

On date Saturday 2011-01-08 19:10:06 -0500, Daniel Kang encoded:
> truemotion2 videos with invalid metadata crashes ffmpeg. There are
> multiple places where the decoder (demuxer?) overreads a buffer. The
> patch adds several checks for this. Are there any comments?

> From fb6f2b4591c059887fa32c5277aade5964b6bc70 Mon Sep 17 00:00:00 2001
> From: Daniel Kang <daniel.d.kang at gmail.com>
> Date: Sat, 8 Jan 2011 16:11:31 -0500
> Subject: [PATCH] Fix several errors in truemotion2 decoding
> 
> ---
>  libavcodec/truemotion2.c |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/truemotion2.c b/libavcodec/truemotion2.c
> index 5013a9e..9c08d61 100644
> --- a/libavcodec/truemotion2.c
> +++ b/libavcodec/truemotion2.c
> @@ -260,7 +260,7 @@ static int tm2_read_deltas(TM2Context *ctx, int stream_id) {
>      return 0;
>  }
> 
> -static int tm2_read_stream(TM2Context *ctx, const uint8_t *buf, int stream_id) {
> +static int tm2_read_stream(TM2Context *ctx, const uint8_t *buf, int stream_id, int buf_size) {

Nit++: "{" on following line per K&R style

>      int i;
>      int cur = 0;
>      int skip = 0;
> @@ -274,6 +274,11 @@ static int tm2_read_stream(TM2Context *ctx, const uint8_t *buf, int stream_id) {
>      if(len == 0)
>          return 4;
> 
> +    if(len >= INT_MAX/4-1 || len < 0 || len > buf_size) {

nit: if_(

> +        av_log(ctx->avctx, AV_LOG_ERROR, "Error, invalid stream size.\n");
> +        return -1;

Please return meaningful error codes, AVERROR_INVALIDDATA in this case.

> +    }
> +
>      toks = AV_RB32(buf); buf += 4; cur += 4;
>      if(toks & 1) {
>          len = AV_RB32(buf); buf += 4; cur += 4;
> @@ -313,8 +318,13 @@ static int tm2_read_stream(TM2Context *ctx, const uint8_t *buf, int stream_id) {
>      len = AV_RB32(buf); buf += 4; cur += 4;
>      if(len > 0) {
>          init_get_bits(&ctx->gb, buf, (skip - cur) * 8);
> -        for(i = 0; i < toks; i++)
> +        for(i = 0; i < toks; i++) {
> +            if (get_bits_left(&ctx->gb) <= 0) {
> +                av_log(ctx->avctx, AV_LOG_ERROR, "Incorrect number of tokens: %i\n", toks);
> +                return -1;

ditto
-- 
FFmpeg = Frenzy and Formidable Miracolous Proud Elected Guru