diff mbox series

[FFmpeg-devel] avcodec/vqavideo: Use GetByteContext and check for end

Message ID 20211113210447.28681-1-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel] avcodec/vqavideo: Use GetByteContext and check for end
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Michael Niedermayer Nov. 13, 2021, 9:04 p.m. UTC
Fixes: out of array access
Fixes: Timeout
Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vqavideo.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Nov. 29, 2021, 2:58 p.m. UTC | #1
On Sat, Nov 13, 2021 at 10:04:47PM +0100, Michael Niedermayer wrote:
> Fixes: out of array access
> Fixes: Timeout
> Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/vqavideo.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

will apply

[...]
Andreas Rheinhardt Nov. 29, 2021, 3 p.m. UTC | #2
Michael Niedermayer:
> Fixes: out of array access
> Fixes: Timeout
> Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/vqavideo.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c
> index 5466e25cdf1..755abf6bafa 100644
> --- a/libavcodec/vqavideo.c
> +++ b/libavcodec/vqavideo.c
> @@ -633,7 +633,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
>      int vptr_chunk = -1;
>      int vprz_chunk = -1;
>  
> -    const unsigned char *stream;
> +    GetByteContext gb_stream;
>  
>      while (bytestream2_get_bytes_left(&s->gb) >= 8) {
>          chunk_type = bytestream2_get_be32u(&s->gb);
> @@ -722,7 +722,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
>  
>      /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */
>  
> -    stream = (unsigned char*)s->decode_buffer;
> +    bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size);
>  
>      for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) {
>          int x_pos = 0;
> @@ -730,9 +730,14 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
>          while (x_pos < s->width) {
>              int vector_index = 0;
>              int count = 0;
> -            uint16_t code = bytestream_get_le16(&stream);
> +            uint16_t code;
>              int type;
>  
> +            if (bytestream2_get_bytes_left(&gb_stream) < 1)

Why are you only checking for one byte to be present although you read
two bytes immediately afterwards?

> +                return AVERROR_INVALIDDATA;
> +
> +            code = bytestream2_get_le16(&gb_stream);
> +
>              type = code >> 13;
>              code &= 0x1fff;
>  
> @@ -747,7 +752,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
>                  count = 1;
>              } else if (type < 7) {
>                  vector_index = code;
> -                count = *stream++;
> +                count = bytestream2_get_byte(&gb_stream);
>              } else {
>                  av_log(s->avctx, AV_LOG_ERROR, " unknown type in VPTR chunk (%d)\n",type);
>                  return AVERROR_INVALIDDATA;
> @@ -776,7 +781,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
>  
>                  /* we might want to read the next block index from stream */
>                  if ((type == 2) && count > 0) {
> -                    vector_index = bytestream_get_byte(&stream);
> +                    vector_index = bytestream2_get_byte(&gb_stream);
>                  }
>  
>                  x_pos += 4;
>
Michael Niedermayer Nov. 29, 2021, 4:12 p.m. UTC | #3
On Mon, Nov 29, 2021 at 04:00:27PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: out of array access
> > Fixes: Timeout
> > Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/vqavideo.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c
> > index 5466e25cdf1..755abf6bafa 100644
> > --- a/libavcodec/vqavideo.c
> > +++ b/libavcodec/vqavideo.c
> > @@ -633,7 +633,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
> >      int vptr_chunk = -1;
> >      int vprz_chunk = -1;
> >  
> > -    const unsigned char *stream;
> > +    GetByteContext gb_stream;
> >  
> >      while (bytestream2_get_bytes_left(&s->gb) >= 8) {
> >          chunk_type = bytestream2_get_be32u(&s->gb);
> > @@ -722,7 +722,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
> >  
> >      /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */
> >  
> > -    stream = (unsigned char*)s->decode_buffer;
> > +    bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size);
> >  
> >      for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) {
> >          int x_pos = 0;
> > @@ -730,9 +730,14 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
> >          while (x_pos < s->width) {
> >              int vector_index = 0;
> >              int count = 0;
> > -            uint16_t code = bytestream_get_le16(&stream);
> > +            uint16_t code;
> >              int type;
> >  
> > +            if (bytestream2_get_bytes_left(&gb_stream) < 1)
> 
> Why are you only checking for one byte to be present although you read
> two bytes immediately afterwards?

because i apparently cannot count to 2
i will fix that before applying
thx


[...]
diff mbox series

Patch

diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c
index 5466e25cdf1..755abf6bafa 100644
--- a/libavcodec/vqavideo.c
+++ b/libavcodec/vqavideo.c
@@ -633,7 +633,7 @@  static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
     int vptr_chunk = -1;
     int vprz_chunk = -1;
 
-    const unsigned char *stream;
+    GetByteContext gb_stream;
 
     while (bytestream2_get_bytes_left(&s->gb) >= 8) {
         chunk_type = bytestream2_get_be32u(&s->gb);
@@ -722,7 +722,7 @@  static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
 
     /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */
 
-    stream = (unsigned char*)s->decode_buffer;
+    bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size);
 
     for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) {
         int x_pos = 0;
@@ -730,9 +730,14 @@  static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
         while (x_pos < s->width) {
             int vector_index = 0;
             int count = 0;
-            uint16_t code = bytestream_get_le16(&stream);
+            uint16_t code;
             int type;
 
+            if (bytestream2_get_bytes_left(&gb_stream) < 1)
+                return AVERROR_INVALIDDATA;
+
+            code = bytestream2_get_le16(&gb_stream);
+
             type = code >> 13;
             code &= 0x1fff;
 
@@ -747,7 +752,7 @@  static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
                 count = 1;
             } else if (type < 7) {
                 vector_index = code;
-                count = *stream++;
+                count = bytestream2_get_byte(&gb_stream);
             } else {
                 av_log(s->avctx, AV_LOG_ERROR, " unknown type in VPTR chunk (%d)\n",type);
                 return AVERROR_INVALIDDATA;
@@ -776,7 +781,7 @@  static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame)
 
                 /* we might want to read the next block index from stream */
                 if ((type == 2) && count > 0) {
-                    vector_index = bytestream_get_byte(&stream);
+                    vector_index = bytestream2_get_byte(&gb_stream);
                 }
 
                 x_pos += 4;