CVE-2017-9990: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Stack-based buffer overflow in the color_string_to_rgba function in libavcodec/xpmdec.c in FFmpeg 3.3 before 3.3.1 allows remote attackers to cause a denial of service (application crash) or possibly have unspecified other impact via a crafted file.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 8.8
Published: 2017-06-28T06:29Z
Repository: https://github.com/FFmpeg/FFmpeg
Language: C
Files changed: libavcodec/xpmdec.c
Commit date: 2017-05-12 23:38:03 +0200
Commit message: avcodec/xpmdec: Fix multiple pointer/memory issues

Most of these were found through code review in response to
fixing 1466/clusterfuzz-testcase-minimized-5961584419536896
There is thus no testcase for most of this.
The initial issue was Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>

Diff: diff --git a/libavcodec/xpmdec.c b/libavcodec/xpmdec.c
index 9112d4cb5e0b..03172e4aadab 100644
--- a/libavcodec/xpmdec.c
+++ b/libavcodec/xpmdec.c
@@ -27,10 +27,12 @@
 #include "internal.h"
 
 typedef struct XPMContext {
     uint32_t  *pixels;
     int        pixels_size;
+    uint8_t   *buf;
+    int        buf_size;
 } XPMDecContext;
 
 typedef struct ColorEntry {
     const char *name;         ///< a string representing the name of the color
     uint32_t    rgb_color;    ///< RGB values for the color
@@ -231,10 +233,12 @@ static uint32_t color_string_to_rgba(const char *p, int len)
 {
     uint32_t ret = 0xFF000000;
     const ColorEntry *entry;
     char color_name[100];
 
+    len = FFMIN(FFMAX(len, 0), sizeof(color_name) - 1);
+
     if (*p == '#') {
         p++;
         len--;
         if (len == 3) {
             ret |= (hex_char_to_number(p[2]) <<  4) |
@@ -297,22 +301,29 @@ static int ascii2index(const uint8_t *cpixel, int cpp)
 static int xpm_decode_frame(AVCodecContext *avctx, void *data,
                             int *got_frame, AVPacket *avpkt)
 {
     XPMDecContext *x = avctx->priv_data;
     AVFrame *p=data;
-    const uint8_t *end, *ptr = avpkt->data;
+    const uint8_t *end, *ptr;
     int ncolors, cpp, ret, i, j;
     int64_t size;
     uint32_t *dst;
 
     avctx->pix_fmt = AV_PIX_FMT_BGRA;
 
-    end = avpkt->data + avpkt->size;
-    while (memcmp(ptr, "/* XPM */", 9) && ptr < end - 9)
+    av_fast_padded_malloc(&x->buf, &x->buf_size, avpkt->size);
+    if (!x->buf)
+        return AVERROR(ENOMEM);
+    memcpy(x->buf, avpkt->data, avpkt->size);
+    x->buf[avpkt->size] = 0;
+
+    ptr = x->buf;
+    end = x->buf + avpkt->size;
+    while (end - ptr > 9 && memcmp(ptr, "/* XPM */", 9))
         ptr++;
 
-    if (ptr >= end) {
+    if (end - ptr <= 9) {
         av_log(avctx, AV_LOG_ERROR, "missing signature\n");
         return AVERROR_INVALIDDATA;
     }
 
     ptr += mod_strcspn(ptr, "\"");
@@ -333,11 +344,11 @@ static int xpm_decode_frame(AVCodecContext *avctx, void *data,
         return AVERROR_INVALIDDATA;
     }
 
     size = 1;
     for (i = 0; i < cpp; i++)
-        size *= 94;
+        size *= 95;
 
     if (ncolors <= 0 || ncolors > size) {
         av_log(avctx, AV_LOG_ERROR, "invalid number of colors: %d\n", ncolors);
         return AVERROR_INVALIDDATA;
     }
@@ -347,16 +358,19 @@ static int xpm_decode_frame(AVCodecContext *avctx, void *data,
     av_fast_padded_malloc(&x->pixels, &x->pixels_size, size);
     if (!x->pixels)
         return AVERROR(ENOMEM);
 
     ptr += mod_strcspn(ptr, ",") + 1;
+    if (end - ptr < 1)
+        return AVERROR_INVALIDDATA;
+
     for (i = 0; i < ncolors; i++) {
         const uint8_t *index;
         int len;
 
         ptr += mod_strcspn(ptr, "\"") + 1;
-        if (ptr + cpp > end)
+        if (end - ptr < cpp)
             return AVERROR_INVALIDDATA;
         index = ptr;
         ptr += cpp;
 
         ptr = strstr(ptr, "c ");
@@ -371,18 +385,24 @@ static int xpm_decode_frame(AVCodecContext *avctx, void *data,
         if ((ret = ascii2index(index, cpp)) < 0)
             return ret;
 
         x->pixels[ret] = color_string_to_rgba(ptr, len);
         ptr += mod_strcspn(ptr, ",") + 1;
+        if (end - ptr < 1)
+            return AVERROR_INVALIDDATA;
     }
 
     for (i = 0; i < avctx->height; i++) {
         dst = (uint32_t *)(p->data[0] + i * p->linesize[0]);
+        if (end - ptr < 1)
+            return AVERROR_INVALIDDATA;
         ptr += mod_strcspn(ptr, "\"") + 1;
+        if (end - ptr < 1)
+            return AVERROR_INVALIDDATA;
 
         for (j = 0; j < avctx->width; j++) {
-            if (ptr + cpp > end)
+            if (end - ptr < cpp)
                 return AVERROR_INVALIDDATA;
 
             if ((ret = ascii2index(ptr, cpp)) < 0)
                 return ret;
 
@@ -403,10 +423,13 @@ static int xpm_decode_frame(AVCodecContext *avctx, void *data,
 static av_cold int xpm_decode_close(AVCodecContext *avctx)
 {
     XPMDecContext *x = avctx->priv_data;
     av_freep(&x->pixels);
 
+    av_freep(&x->buf);
+    x->buf_size = 0;
+
     return 0;
 }
 
 AVCodec ff_xpm_decoder = {
     .name           = "xpm",

Vulnerable code:     const uint8_t *end, *ptr = avpkt->data;
    end = avpkt->data + avpkt->size;
    while (memcmp(ptr, "/* XPM */", 9) && ptr < end - 9)
    if (ptr >= end) {
        size *= 94;
        if (ptr + cpp > end)
            if (ptr + cpp > end)
Fixed code:     uint8_t   *buf;
    int        buf_size;
    len = FFMIN(FFMAX(len, 0), sizeof(color_name) - 1);

    const uint8_t *end, *ptr;
    av_fast_padded_malloc(&x->buf, &x->buf_size, avpkt->size);
    if (!x->buf)
        return AVERROR(ENOMEM);
    memcpy(x->buf, avpkt->data, avpkt->size);
    x->buf[avpkt->size] = 0;

    ptr = x->buf;
    end = x->buf + avpkt->size;
    while (end - ptr > 9 && memcmp(ptr, "/* XPM */", 9))
    if (end - ptr <= 9) {
        size *= 95;
    if (end - ptr < 1)
        return AVERROR_INVALIDDATA;

        if (end - ptr < cpp)
        if (end - ptr < 1)
            return AVERROR_INVALIDDATA;
        if (end - ptr < 1)
            return AVERROR_INVALIDDATA;
        if (end - ptr < 1)
            return AVERROR_INVALIDDATA;
            if (end - ptr < cpp)
    av_freep(&x->buf);
    x->buf_size = 0;