sgidec: Use bytestream2 functions to prevent buffer overreads.

The patch also adds several bytestream macros to deal with native endian.
Signed-off-by: Justin Ruggles <justin.ruggles@gmail.com>

libavcodec/bytestream.h

@@ -75,6 +75,42 @@ DEF  (byte, 1, AV_RB8 , AV_WB8 )
 #undef DEF64
 #undef DEF_T
 
+#if HAVE_BIGENDIAN
+#   define bytestream2_get_ne16  bytestream2_get_be16
+#   define bytestream2_get_ne24  bytestream2_get_be24
+#   define bytestream2_get_ne32  bytestream2_get_be32
+#   define bytestream2_get_ne64  bytestream2_get_be64
+#   define bytestream2_get_ne16u bytestream2_get_be16u
+#   define bytestream2_get_ne24u bytestream2_get_be24u
+#   define bytestream2_get_ne32u bytestream2_get_be32u
+#   define bytestream2_get_ne64u bytestream2_get_be64u
+#   define bytestream2_put_ne16  bytestream2_put_be16
+#   define bytestream2_put_ne24  bytestream2_put_be24
+#   define bytestream2_put_ne32  bytestream2_put_be32
+#   define bytestream2_put_ne64  bytestream2_put_be64
+#   define bytestream2_peek_ne16 bytestream2_peek_be16
+#   define bytestream2_peek_ne24 bytestream2_peek_be24
+#   define bytestream2_peek_ne32 bytestream2_peek_be32
+#   define bytestream2_peek_ne64 bytestream2_peek_be64
+#else
+#   define bytestream2_get_ne16  bytestream2_get_le16
+#   define bytestream2_get_ne24  bytestream2_get_le24
+#   define bytestream2_get_ne32  bytestream2_get_le32
+#   define bytestream2_get_ne64  bytestream2_get_le64
+#   define bytestream2_get_ne16u bytestream2_get_le16u
+#   define bytestream2_get_ne24u bytestream2_get_le24u
+#   define bytestream2_get_ne32u bytestream2_get_le32u
+#   define bytestream2_get_ne64u bytestream2_get_le64u
+#   define bytestream2_put_ne16  bytestream2_put_le16
+#   define bytestream2_put_ne24  bytestream2_put_le24
+#   define bytestream2_put_ne32  bytestream2_put_le32
+#   define bytestream2_put_ne64  bytestream2_put_le64
+#   define bytestream2_peek_ne16 bytestream2_peek_le16
+#   define bytestream2_peek_ne24 bytestream2_peek_le24
+#   define bytestream2_peek_ne32 bytestream2_peek_le32
+#   define bytestream2_peek_ne64 bytestream2_peek_le64
+#endif
+
 static av_always_inline void bytestream2_init(GetByteContext *g,
                                               const uint8_t *buf, int buf_size)
 {

libavcodec/sgidec.c

@@ -31,26 +31,27 @@ typedef struct SgiState {
     unsigned int depth;
     unsigned int bytes_per_channel;
     int linesize;
+    GetByteContext g;
 } SgiState;
 
 /**
  * Expand an RLE row into a channel.
- * @param in_buf input buffer
- * @param in_end end of input buffer
+ * @param s the current image state
  * @param out_buf Points to one line after the output buffer.
  * @param out_end end of line in output buffer
  * @param pixelstride pixel stride of input buffer
  * @return size of output in bytes, -1 if buffer overflows
  */
-static int expand_rle_row(const uint8_t *in_buf, const uint8_t* in_end,
-            unsigned char *out_buf, uint8_t* out_end, int pixelstride)
+static int expand_rle_row(SgiState *s, uint8_t *out_buf,
+                          uint8_t *out_end, int pixelstride)
 {
     unsigned char pixel, count;
     unsigned char *orig = out_buf;
 
     while (1) {
-        if(in_buf + 1 > in_end) return -1;
-        pixel = bytestream_get_byte(&in_buf);
+        if (bytestream2_get_bytes_left(&s->g) < 1)
+            return AVERROR_INVALIDDATA;
+        pixel = bytestream2_get_byteu(&s->g);
         if (!(count = (pixel & 0x7f))) {
             return (out_buf - orig) / pixelstride;
         }
@@ -60,11 +61,11 @@ static int expand_rle_row(const uint8_t *in_buf, const uint8_t* in_end,
 
         if (pixel & 0x80) {
             while (count--) {
-                *out_buf = bytestream_get_byte(&in_buf);
+                *out_buf = bytestream2_get_byte(&s->g);
                 out_buf += pixelstride;
             }
         } else {
-            pixel = bytestream_get_byte(&in_buf);
+            pixel = bytestream2_get_byte(&s->g);
 
             while (count--) {
                 *out_buf = pixel;
@@ -77,37 +78,32 @@ static int expand_rle_row(const uint8_t *in_buf, const uint8_t* in_end,
 /**
  * Read a run length encoded SGI image.
  * @param out_buf output buffer
- * @param in_buf input buffer
- * @param in_end end of input buffer
  * @param s the current image state
  * @return 0 if no error, else return error number.
  */
-static int read_rle_sgi(unsigned char* out_buf, const uint8_t *in_buf,
-                        const uint8_t *in_end, SgiState* s)
+static int read_rle_sgi(uint8_t *out_buf, SgiState *s)
 {
     uint8_t *dest_row;
     unsigned int len = s->height * s->depth * 4;
-    const uint8_t *start_table = in_buf;
+    GetByteContext g_table = s->g;
     unsigned int y, z;
     unsigned int start_offset;
 
     /* size of  RLE offset and length tables */
-    if(len * 2  > in_end - in_buf) {
+    if (len * 2  > bytestream2_get_bytes_left(&s->g)) {
         return AVERROR_INVALIDDATA;
     }
 
-    in_buf -= SGI_HEADER_SIZE;
     for (z = 0; z < s->depth; z++) {
         dest_row = out_buf;
         for (y = 0; y < s->height; y++) {
             dest_row -= s->linesize;
-            start_offset = bytestream_get_be32(&start_table);
-            if(start_offset > in_end - in_buf) {
+            start_offset = bytestream2_get_be32(&g_table);
+            bytestream2_seek(&s->g, start_offset, SEEK_SET);
+            if (expand_rle_row(s, dest_row + z, dest_row + FFABS(s->linesize),
+                               s->depth) != s->width) {
                 return AVERROR_INVALIDDATA;
             }
-            if (expand_rle_row(in_buf + start_offset, in_end, dest_row + z,
-                dest_row + FFABS(s->linesize), s->depth) != s->width)
-                return AVERROR_INVALIDDATA;
         }
     }
     return 0;
@@ -117,32 +113,37 @@ static int read_rle_sgi(unsigned char* out_buf, const uint8_t *in_buf,
  * Read an uncompressed SGI image.
  * @param out_buf output buffer
  * @param out_end end ofoutput buffer
- * @param in_buf input buffer
- * @param in_end end of input buffer
  * @param s the current image state
  * @return 0 if read success, otherwise return -1.
  */
 static int read_uncompressed_sgi(unsigned char* out_buf, uint8_t* out_end,
-                const uint8_t *in_buf, const uint8_t *in_end, SgiState* s)
+                                 SgiState *s)
 {
     int x, y, z;
-    const uint8_t *ptr;
     unsigned int offset = s->height * s->width * s->bytes_per_channel;
+    GetByteContext gp[4];
 
     /* Test buffer size. */
-    if (offset * s->depth > in_end - in_buf) {
-       return -1;
+    if (offset * s->depth > bytestream2_get_bytes_left(&s->g))
+        return AVERROR_INVALIDDATA;
+
+    /* Create a reader for each plane */
+    for (z = 0; z < s->depth; z++) {
+        gp[z] = s->g;
+        bytestream2_skip(&gp[z], z * offset);
     }
 
     for (y = s->height - 1; y >= 0; y--) {
         out_end = out_buf + (y * s->linesize);
-        for (x = s->width; x > 0; x--) {
-            ptr = in_buf += s->bytes_per_channel;
-            for(z = 0; z < s->depth; z ++) {
-                memcpy(out_end, ptr, s->bytes_per_channel);
-                out_end += s->bytes_per_channel;
-                ptr += offset;
-            }
+        if (s->bytes_per_channel == 1) {
+            for (x = s->width; x > 0; x--)
+                for (z = 0; z < s->depth; z++)
+                    *out_end++ = bytestream2_get_byteu(&gp[z]);
+        } else {
+            uint16_t *out16 = (uint16_t *)out_end;
+            for (x = s->width; x > 0; x--)
+                for (z = 0; z < s->depth; z++)
+                    *out16++ = bytestream2_get_ne16u(&gp[z]);
         }
     }
     return 0;
@@ -152,33 +153,31 @@ static int decode_frame(AVCodecContext *avctx,
                         void *data, int *data_size,
                         AVPacket *avpkt)
 {
-    const uint8_t *in_buf = avpkt->data;
-    int buf_size = avpkt->size;
     SgiState *s = avctx->priv_data;
     AVFrame *picture = data;
     AVFrame *p = &s->picture;
-    const uint8_t *in_end = in_buf + buf_size;
     unsigned int dimension, rle;
     int ret = 0;
     uint8_t *out_buf, *out_end;
 
-    if (buf_size < SGI_HEADER_SIZE){
-        av_log(avctx, AV_LOG_ERROR, "buf_size too small (%d)\n", buf_size);
-        return -1;
+    bytestream2_init(&s->g, avpkt->data, avpkt->size);
+    if (bytestream2_get_bytes_left(&s->g) < SGI_HEADER_SIZE) {
+        av_log(avctx, AV_LOG_ERROR, "buf_size too small (%d)\n", avpkt->size);
+        return AVERROR_INVALIDDATA;
     }
 
     /* Test for SGI magic. */
-    if (bytestream_get_be16(&in_buf) != SGI_MAGIC) {
+    if (bytestream2_get_be16(&s->g) != SGI_MAGIC) {
         av_log(avctx, AV_LOG_ERROR, "bad magic number\n");
-        return -1;
+        return AVERROR_INVALIDDATA;
     }
 
-    rle = bytestream_get_byte(&in_buf);
-    s->bytes_per_channel = bytestream_get_byte(&in_buf);
-    dimension = bytestream_get_be16(&in_buf);
-    s->width  = bytestream_get_be16(&in_buf);
-    s->height = bytestream_get_be16(&in_buf);
-    s->depth  = bytestream_get_be16(&in_buf);
+    rle                  = bytestream2_get_byte(&s->g);
+    s->bytes_per_channel = bytestream2_get_byte(&s->g);
+    dimension            = bytestream2_get_be16(&s->g);
+    s->width             = bytestream2_get_be16(&s->g);
+    s->height            = bytestream2_get_be16(&s->g);
+    s->depth             = bytestream2_get_be16(&s->g);
 
     if (s->bytes_per_channel != 1 && (s->bytes_per_channel != 2 || rle)) {
         av_log(avctx, AV_LOG_ERROR, "wrong channel number\n");
@@ -224,19 +223,19 @@ static int decode_frame(AVCodecContext *avctx,
     s->linesize = p->linesize[0];
 
     /* Skip header. */
-    in_buf += SGI_HEADER_SIZE - 12;
+    bytestream2_seek(&s->g, SGI_HEADER_SIZE, SEEK_SET);
     if (rle) {
-        ret = read_rle_sgi(out_end, in_buf, in_end, s);
+        ret = read_rle_sgi(out_end, s);
     } else {
-        ret = read_uncompressed_sgi(out_buf, out_end, in_buf, in_end, s);
+        ret = read_uncompressed_sgi(out_buf, out_end, s);
     }
 
     if (ret == 0) {
         *picture   = s->picture;
         *data_size = sizeof(AVPicture);
-        return buf_size;
+        return avpkt->size;
     } else {
-        return -1;
+        return ret;
     }
 }