diff mbox

[FFmpeg-devel] avcodec/wmv2dec: Check end of bitstream in parse_mb_skip() and ff_wmv2_decode_mb()

Message ID 20171026104730.6404-1-michael@niedermayer.cc
State Superseded
Headers show

Commit Message

Michael Niedermayer Oct. 26, 2017, 10:47 a.m. UTC 
Fixes: Timeout
Fixes: 3200/clusterfuzz-testcase-5750022136135680

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/wmv2dec.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Derek Buitenhuis Oct. 26, 2017, 1:20 p.m. UTC | #1 
On 10/26/2017 11:47 AM, Michael Niedermayer wrote:
> +    if (get_bits_left(&s->gb) < 0) {
> +        return AVERROR_INVALIDDATA;
> +    }

Is this possible? I don't see where get_bits.h is include
in this (probably deep in some other header), so can't see
if it's using the unchecked reader.

- Derek
Michael Niedermayer Oct. 27, 2017, 7:25 p.m. UTC | #2 
On Thu, Oct 26, 2017 at 02:20:28PM +0100, Derek Buitenhuis wrote:
> On 10/26/2017 11:47 AM, Michael Niedermayer wrote:
> > +    if (get_bits_left(&s->gb) < 0) {
> > +        return AVERROR_INVALIDDATA;
> > +    }
> 
> Is this possible? I don't see where get_bits.h is include
> in this (probably deep in some other header), so can't see
> if it's using the unchecked reader.

get_bits.h is included from libavcodec/mpegvideo.h as it needs it

ill send a better patch

thx


[...]
diff mbox

Patch

diff --git a/libavcodec/wmv2dec.c b/libavcodec/wmv2dec.c
index 261d291c97..bfde13bacb 100644
--- a/libavcodec/wmv2dec.c
+++ b/libavcodec/wmv2dec.c
@@ -30,7 +30,7 @@ 
 #include "wmv2.h"
 
 
-static void parse_mb_skip(Wmv2Context *w)
+static int parse_mb_skip(Wmv2Context *w)
 {
     int mb_x, mb_y;
     MpegEncContext *const s = &w->s;
@@ -45,6 +45,8 @@  static void parse_mb_skip(Wmv2Context *w)
                     MB_TYPE_16x16 | MB_TYPE_L0;
         break;
     case SKIP_TYPE_MPEG:
+        if (get_bits_left(&s->gb) < s->mb_height * s->mb_width)
+            return AVERROR_INVALIDDATA;
         for (mb_y = 0; mb_y < s->mb_height; mb_y++)
             for (mb_x = 0; mb_x < s->mb_width; mb_x++)
                 mb_type[mb_y * s->mb_stride + mb_x] =
@@ -52,6 +54,8 @@  static void parse_mb_skip(Wmv2Context *w)
         break;
     case SKIP_TYPE_ROW:
         for (mb_y = 0; mb_y < s->mb_height; mb_y++) {
+            if (get_bits_left(&s->gb) < 1)
+                return AVERROR_INVALIDDATA;
             if (get_bits1(&s->gb)) {
                 for (mb_x = 0; mb_x < s->mb_width; mb_x++)
                     mb_type[mb_y * s->mb_stride + mb_x] =
@@ -65,6 +69,8 @@  static void parse_mb_skip(Wmv2Context *w)
         break;
     case SKIP_TYPE_COL:
         for (mb_x = 0; mb_x < s->mb_width; mb_x++) {
+            if (get_bits_left(&s->gb) < 1)
+                return AVERROR_INVALIDDATA;
             if (get_bits1(&s->gb)) {
                 for (mb_y = 0; mb_y < s->mb_height; mb_y++)
                     mb_type[mb_y * s->mb_stride + mb_x] =
@@ -77,6 +83,7 @@  static void parse_mb_skip(Wmv2Context *w)
         }
         break;
     }
+    return 0;
 }
 
 static int decode_ext_header(Wmv2Context *w)
@@ -170,9 +177,12 @@  int ff_wmv2_decode_secondary_picture_header(MpegEncContext *s)
         }
     } else {
         int cbp_index;
+        int ret;
         w->j_type = 0;
 
-        parse_mb_skip(w);
+        ret = parse_mb_skip(w);
+        if (ret < 0)
+            return ret;
         cbp_index = decode012(&s->gb);
         w->cbp_table_index = wmv2_get_cbp_table_index(s, cbp_index);
 
@@ -345,6 +355,10 @@  int ff_wmv2_decode_mb(MpegEncContext *s, int16_t block[6][64])
     if (w->j_type)
         return 0;
 
+    if (get_bits_left(&s->gb) < 0) {
+        return AVERROR_INVALIDDATA;
+    }
+
     if (s->pict_type == AV_PICTURE_TYPE_P) {
         if (IS_SKIP(s->current_picture.mb_type[s->mb_y * s->mb_stride + s->mb_x])) {
             /* skip mb */