[FFmpeg-devel] [PATCH] avcodec/eatgq: prevent out of bounds memory access and endless loop

[Andreas Rheinhardt]

Peter Ross:
> ---
>  libavcodec/eatgq.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/eatgq.c b/libavcodec/eatgq.c
> index 89e9f20880..fdda8286ef 100644
> --- a/libavcodec/eatgq.c
> +++ b/libavcodec/eatgq.c
> @@ -56,7 +56,7 @@ static av_cold int tgq_decode_init(AVCodecContext *avctx)
>      return 0;
>  }
>  
> -static void tgq_decode_block(TgqContext *s, int16_t block[64], GetBitContext *gb)
> +static int tgq_decode_block(TgqContext *s, int16_t block[64], GetBitContext *gb)
>  {
>      const uint8_t *scantable = ff_zigzag_direct;
>      int i, j, value;
> @@ -73,7 +73,9 @@ static void tgq_decode_block(TgqContext *s, int16_t block[64], GetBitContext *gb
>          case 1:
>              skip_bits(gb, 2);
>              value = get_bits(gb, 6);
> -            for (j = 0; j < value; j++)
> +            if (!value)
> +                return AVERROR_INVALIDDATA;
> +            for (j = 0; j < value && i < 64; j++)
>                  block[scantable[i++]] = 0;
>              break;
>          case 6:
> @@ -100,6 +102,7 @@ static void tgq_decode_block(TgqContext *s, int16_t block[64], GetBitContext *gb
>          }
>      }
>      block[0] += 128 << 4;
> +    return 0;
>  }
>  
>  static void tgq_idct_put_mb(TgqContext *s, int16_t (*block)[64], AVFrame *frame,
> @@ -161,7 +164,8 @@ static int tgq_decode_mb(TgqContext *s, GetByteContext *gbyte,
>              return ret;
>  
>          for (i = 0; i < 6; i++)
> -            tgq_decode_block(s, s->block[i], &gb);
> +            if ((ret = tgq_decode_block(s, s->block[i], &gb)) < 0)
> +                return ret;
>          tgq_idct_put_mb(s, s->block, frame, mb_x, mb_y);
>          bytestream2_skip(gbyte, mode);
>      } else {
> 
> 

The '4' case can also overread. But actually I don't like the idea of
adding further checks into the main loop; mind if I send an alternative
solution?

- Andreas