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

[Peter Ross <pross@xvid.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2222 bytes --]

On Wed, Oct 26, 2022 at 01:41:57PM +0200, Andreas Rheinhardt wrote:
> 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?

please go for it.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".