kmvc: Use bytestream2 functions to prevent buffer overreads.

Signed-off-by: Ronald S. Bultje <rsbultje@gmail.com>

1 files changed, 58 insertions, 92 deletions

diff --git a/libavcodec/kmvc.c b/libavcodec/kmvc.c
index 6c55863e7d..2b54b84e99 100644
--- a/libavcodec/kmvc.c
+++ b/libavcodec/kmvc.c
@@ -46,6 +46,7 @@ typedef struct KmvcContext {
     uint32_t pal[256];
     uint8_t *cur, *prev;
     uint8_t *frm0, *frm1;
+    GetByteContext g;
 } KmvcContext;
 
 typedef struct BitBuf {
@@ -55,23 +56,19 @@ typedef struct BitBuf {
 
 #define BLK(data, x, y)  data[(x) + (y) * 320]
 
-#define kmvc_init_getbits(bb, src)  bb.bits = 7; bb.bitbuf = *src++;
+#define kmvc_init_getbits(bb, g)  bb.bits = 7; bb.bitbuf = bytestream2_get_byte(g);
 
-#define kmvc_getbit(bb, src, src_end, res) {\
+#define kmvc_getbit(bb, g, res) {\
     res = 0; \
     if (bb.bitbuf & (1 << bb.bits)) res = 1; \
     bb.bits--; \
     if(bb.bits == -1) { \
-        if (src >= src_end) { \
-            av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n"); \
-            return AVERROR_INVALIDDATA; \
-        } \
-        bb.bitbuf = *src++; \
+        bb.bitbuf = bytestream2_get_byte(g); \
         bb.bits = 7; \
     } \
 }
 
-static int kmvc_decode_intra_8x8(KmvcContext * ctx, const uint8_t * src, int src_size, int w, int h)
+static int kmvc_decode_intra_8x8(KmvcContext * ctx, int w, int h)
 {
     BitBuf bb;
     int res, val;
@@ -79,42 +76,33 @@ static int kmvc_decode_intra_8x8(KmvcContext * ctx, const uint8_t * src, int src
     int bx, by;
     int l0x, l1x, l0y, l1y;
     int mx, my;
-    const uint8_t *src_end = src + src_size;
 
-    kmvc_init_getbits(bb, src);
+    kmvc_init_getbits(bb, &ctx->g);
 
     for (by = 0; by < h; by += 8)
         for (bx = 0; bx < w; bx += 8) {
-            kmvc_getbit(bb, src, src_end, res);
+            if (!bytestream2_get_bytes_left(&ctx->g)) {
+                av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
+                return AVERROR_INVALIDDATA;
+            }
+            kmvc_getbit(bb, &ctx->g, res);
             if (!res) {         // fill whole 8x8 block
-                if (src >= src_end) {
-                    av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                    return AVERROR_INVALIDDATA;
-                }
-                val = *src++;
+                val = bytestream2_get_byte(&ctx->g);
                 for (i = 0; i < 64; i++)
                     BLK(ctx->cur, bx + (i & 0x7), by + (i >> 3)) = val;
             } else {            // handle four 4x4 subblocks
                 for (i = 0; i < 4; i++) {
                     l0x = bx + (i & 1) * 4;
                     l0y = by + (i & 2) * 2;
-                    kmvc_getbit(bb, src, src_end, res);
+                    kmvc_getbit(bb, &ctx->g, res);
                     if (!res) {
-                        kmvc_getbit(bb, src, src_end, res);
+                        kmvc_getbit(bb, &ctx->g, res);
                         if (!res) {     // fill whole 4x4 block
-                            if (src >= src_end) {
-                                av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                return AVERROR_INVALIDDATA;
-                            }
-                            val = *src++;
+                            val = bytestream2_get_byte(&ctx->g);
                             for (j = 0; j < 16; j++)
                                 BLK(ctx->cur, l0x + (j & 3), l0y + (j >> 2)) = val;
                         } else {        // copy block from already decoded place
-                            if (src >= src_end) {
-                                av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                return AVERROR_INVALIDDATA;
-                            }
-                            val = *src++;
+                            val = bytestream2_get_byte(&ctx->g);
                             mx = val & 0xF;
                             my = val >> 4;
                             for (j = 0; j < 16; j++)
@@ -125,25 +113,17 @@ static int kmvc_decode_intra_8x8(KmvcContext * ctx, const uint8_t * src, int src
                         for (j = 0; j < 4; j++) {
                             l1x = l0x + (j & 1) * 2;
                             l1y = l0y + (j & 2);
-                            kmvc_getbit(bb, src, src_end, res);
+                            kmvc_getbit(bb, &ctx->g, res);
                             if (!res) {
-                                kmvc_getbit(bb, src, src_end, res);
+                                kmvc_getbit(bb, &ctx->g, res);
                                 if (!res) {     // fill whole 2x2 block
-                                    if (src >= src_end) {
-                                        av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                        return AVERROR_INVALIDDATA;
-                                    }
-                                    val = *src++;
+                                    val = bytestream2_get_byte(&ctx->g);
                                     BLK(ctx->cur, l1x, l1y) = val;
                                     BLK(ctx->cur, l1x + 1, l1y) = val;
                                     BLK(ctx->cur, l1x, l1y + 1) = val;
                                     BLK(ctx->cur, l1x + 1, l1y + 1) = val;
                                 } else {        // copy block from already decoded place
-                                    if (src >= src_end) {
-                                        av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                        return AVERROR_INVALIDDATA;
-                                    }
-                                    val = *src++;
+                                    val = bytestream2_get_byte(&ctx->g);
                                     mx = val & 0xF;
                                     my = val >> 4;
                                     BLK(ctx->cur, l1x, l1y) = BLK(ctx->cur, l1x - mx, l1y - my);
@@ -155,10 +135,10 @@ static int kmvc_decode_intra_8x8(KmvcContext * ctx, const uint8_t * src, int src
                                         BLK(ctx->cur, l1x + 1 - mx, l1y + 1 - my);
                                 }
                             } else {    // read values for block
-                                BLK(ctx->cur, l1x, l1y) = *src++;
-                                BLK(ctx->cur, l1x + 1, l1y) = *src++;
-                                BLK(ctx->cur, l1x, l1y + 1) = *src++;
-                                BLK(ctx->cur, l1x + 1, l1y + 1) = *src++;
+                                BLK(ctx->cur, l1x, l1y) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x + 1, l1y) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x, l1y + 1) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x + 1, l1y + 1) = bytestream2_get_byte(&ctx->g);
                             }
                         }
                     }
@@ -169,7 +149,7 @@ static int kmvc_decode_intra_8x8(KmvcContext * ctx, const uint8_t * src, int src
     return 0;
 }
 
-static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src_size, int w, int h)
+static int kmvc_decode_inter_8x8(KmvcContext * ctx, int w, int h)
 {
     BitBuf bb;
     int res, val;
@@ -177,21 +157,20 @@ static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src
     int bx, by;
     int l0x, l1x, l0y, l1y;
     int mx, my;
-    const uint8_t *src_end = src + src_size;
 
-    kmvc_init_getbits(bb, src);
+    kmvc_init_getbits(bb, &ctx->g);
 
     for (by = 0; by < h; by += 8)
         for (bx = 0; bx < w; bx += 8) {
-            kmvc_getbit(bb, src, src_end, res);
+            kmvc_getbit(bb, &ctx->g, res);
             if (!res) {
-                kmvc_getbit(bb, src, src_end, res);
+                kmvc_getbit(bb, &ctx->g, res);
                 if (!res) {     // fill whole 8x8 block
-                    if (src >= src_end) {
+                    if (!bytestream2_get_bytes_left(&ctx->g)) {
                         av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
                         return AVERROR_INVALIDDATA;
                     }
-                    val = *src++;
+                    val = bytestream2_get_byte(&ctx->g);
                     for (i = 0; i < 64; i++)
                         BLK(ctx->cur, bx + (i & 0x7), by + (i >> 3)) = val;
                 } else {        // copy block from previous frame
@@ -200,26 +179,22 @@ static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src
                             BLK(ctx->prev, bx + (i & 0x7), by + (i >> 3));
                 }
             } else {            // handle four 4x4 subblocks
+                if (!bytestream2_get_bytes_left(&ctx->g)) {
+                    av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
+                    return AVERROR_INVALIDDATA;
+                }
                 for (i = 0; i < 4; i++) {
                     l0x = bx + (i & 1) * 4;
                     l0y = by + (i & 2) * 2;
-                    kmvc_getbit(bb, src, src_end, res);
+                    kmvc_getbit(bb, &ctx->g, res);
                     if (!res) {
-                        kmvc_getbit(bb, src, src_end, res);
+                        kmvc_getbit(bb, &ctx->g, res);
                         if (!res) {     // fill whole 4x4 block
-                            if (src >= src_end) {
-                                av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                return AVERROR_INVALIDDATA;
-                            }
-                            val = *src++;
+                            val = bytestream2_get_byte(&ctx->g);
                             for (j = 0; j < 16; j++)
                                 BLK(ctx->cur, l0x + (j & 3), l0y + (j >> 2)) = val;
                         } else {        // copy block
-                            if (src >= src_end) {
-                                av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                return AVERROR_INVALIDDATA;
-                            }
-                            val = *src++;
+                            val = bytestream2_get_byte(&ctx->g);
                             mx = (val & 0xF) - 8;
                             my = (val >> 4) - 8;
                             for (j = 0; j < 16; j++)
@@ -230,25 +205,17 @@ static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src
                         for (j = 0; j < 4; j++) {
                             l1x = l0x + (j & 1) * 2;
                             l1y = l0y + (j & 2);
-                            kmvc_getbit(bb, src, src_end, res);
+                            kmvc_getbit(bb, &ctx->g, res);
                             if (!res) {
-                                kmvc_getbit(bb, src, src_end, res);
+                                kmvc_getbit(bb, &ctx->g, res);
                                 if (!res) {     // fill whole 2x2 block
-                                    if (src >= src_end) {
-                                        av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                        return AVERROR_INVALIDDATA;
-                                    }
-                                    val = *src++;
+                                    val = bytestream2_get_byte(&ctx->g);
                                     BLK(ctx->cur, l1x, l1y) = val;
                                     BLK(ctx->cur, l1x + 1, l1y) = val;
                                     BLK(ctx->cur, l1x, l1y + 1) = val;
                                     BLK(ctx->cur, l1x + 1, l1y + 1) = val;
                                 } else {        // copy block
-                                    if (src >= src_end) {
-                                        av_log(ctx->avctx, AV_LOG_ERROR, "Data overrun\n");
-                                        return AVERROR_INVALIDDATA;
-                                    }
-                                    val = *src++;
+                                    val = bytestream2_get_byte(&ctx->g);
                                     mx = (val & 0xF) - 8;
                                     my = (val >> 4) - 8;
                                     BLK(ctx->cur, l1x, l1y) = BLK(ctx->prev, l1x + mx, l1y + my);
@@ -260,10 +227,10 @@ static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src
                                         BLK(ctx->prev, l1x + 1 + mx, l1y + 1 + my);
                                 }
                             } else {    // read values for block
-                                BLK(ctx->cur, l1x, l1y) = *src++;
-                                BLK(ctx->cur, l1x + 1, l1y) = *src++;
-                                BLK(ctx->cur, l1x, l1y + 1) = *src++;
-                                BLK(ctx->cur, l1x + 1, l1y + 1) = *src++;
+                                BLK(ctx->cur, l1x, l1y) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x + 1, l1y) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x, l1y + 1) = bytestream2_get_byte(&ctx->g);
+                                BLK(ctx->cur, l1x + 1, l1y + 1) = bytestream2_get_byte(&ctx->g);
                             }
                         }
                     }
@@ -276,8 +243,6 @@ static int kmvc_decode_inter_8x8(KmvcContext * ctx, const uint8_t * src, int src
 
 static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPacket *avpkt)
 {
-    const uint8_t *buf = avpkt->data;
-    int buf_size = avpkt->size;
     KmvcContext *const ctx = avctx->priv_data;
     uint8_t *out, *src;
     int i;
@@ -285,6 +250,7 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
     int blocksize;
     const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, NULL);
 
+    bytestream2_init(&ctx->g, avpkt->data, avpkt->size);
     if (ctx->pic.data[0])
         avctx->release_buffer(avctx, &ctx->pic);
 
@@ -295,16 +261,16 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
         return -1;
     }
 
-    header = *buf++;
+    header = bytestream2_get_byte(&ctx->g);
 
     /* blocksize 127 is really palette change event */
-    if (buf[0] == 127) {
-        buf += 3;
+    if (bytestream2_peek_byte(&ctx->g) == 127) {
+        bytestream2_skip(&ctx->g, 3);
         for (i = 0; i < 127; i++) {
-            ctx->pal[i + (header & 0x81)] = AV_RB24(buf);
-            buf += 4;
+            ctx->pal[i + (header & 0x81)] = bytestream2_get_be24(&ctx->g);
+            bytestream2_skip(&ctx->g, 1);
         }
-        buf -= 127 * 4 + 3;
+        bytestream2_seek(&ctx->g, -127 * 4 - 3, SEEK_CUR);
     }
 
     if (header & KMVC_KEYFRAME) {
@@ -319,7 +285,7 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
         ctx->pic.palette_has_changed = 1;
         // palette starts from index 1 and has 127 entries
         for (i = 1; i <= ctx->palsize; i++) {
-            ctx->pal[i] = bytestream_get_be24(&buf);
+            ctx->pal[i] = bytestream2_get_be24(&ctx->g);
         }
     }
 
@@ -336,7 +302,7 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
     /* make the palette available on the way out */
     memcpy(ctx->pic.data[1], ctx->pal, 1024);
 
-    blocksize = *buf++;
+    blocksize = bytestream2_get_byte(&ctx->g);
 
     if (blocksize != 8 && blocksize != 127) {
         av_log(avctx, AV_LOG_ERROR, "Block size = %i\n", blocksize);
@@ -349,10 +315,10 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
         memcpy(ctx->cur, ctx->prev, 320 * 200);
         break;
     case 3:
-        kmvc_decode_intra_8x8(ctx, buf, buf_size, avctx->width, avctx->height);
+        kmvc_decode_intra_8x8(ctx, avctx->width, avctx->height);
         break;
     case 4:
-        kmvc_decode_inter_8x8(ctx, buf, buf_size, avctx->width, avctx->height);
+        kmvc_decode_inter_8x8(ctx, avctx->width, avctx->height);
         break;
     default:
         av_log(avctx, AV_LOG_ERROR, "Unknown compression method %i\n", header & KMVC_METHOD);
@@ -380,7 +346,7 @@ static int decode_frame(AVCodecContext * avctx, void *data, int *data_size, AVPa
     *(AVFrame *) data = ctx->pic;
 
     /* always report that the buffer was completely consumed */
-    return buf_size;
+    return avpkt->size;
 }