Fixed TALOS-2019-0844 - XPM image colorhash parsing Code Execution Vu…

…lnerability The table entry in the color_hash is created in the create_colorhash function based on the number of colors passed into the function. The size of the color_hash table is the first value in the powers of 2 larger than the passed in number of colors [2]. The size of the allocation is this calculated value * 8 (sizeof(struct hash_entry **)) [3]. This multiplication can cause an overflow, resulting in a very small allocation.
libsdl-org · Jun 11, 2019 · 585b177 · 585b177
1 parent 52b9d17
commit 585b177
Showing 1 changed file with 19 additions and 5 deletions.
diff --git a/IMG_xpm.c b/IMG_xpm.c
@@ -101,7 +101,7 @@ static struct color_hash *create_colorhash(int maxnum)
 
     /* we know how many entries we need, so we can allocate
        everything here */
-    hash = (struct color_hash *)SDL_malloc(sizeof *hash);
+    hash = (struct color_hash *)SDL_calloc(1, sizeof(*hash));
     if (!hash)
         return NULL;
 
@@ -110,15 +110,29 @@ static struct color_hash *create_colorhash(int maxnum)
         ;
     hash->size = s;
     hash->maxnum = maxnum;
+
     bytes = hash->size * sizeof(struct hash_entry **);
-    hash->entries = NULL;   /* in case malloc fails */
-    hash->table = (struct hash_entry **)SDL_malloc(bytes);
+    /* Check for overflow */
+    if ((bytes / sizeof(struct hash_entry **)) != hash->size) {
+        IMG_SetError("memory allocation overflow");
+        SDL_free(hash);
+        return NULL;
+    }
+    hash->table = (struct hash_entry **)SDL_calloc(1, bytes);
     if (!hash->table) {
         SDL_free(hash);
         return NULL;
     }
-    SDL_memset(hash->table, 0, bytes);
-    hash->entries = (struct hash_entry *)SDL_malloc(maxnum * sizeof(struct hash_entry));
+
+    bytes = maxnum * sizeof(struct hash_entry);
+    /* Check for overflow */
+    if ((bytes / sizeof(struct hash_entry)) != maxnum) {
+        IMG_SetError("memory allocation overflow");
+        SDL_free(hash->table);
+        SDL_free(hash);
+        return NULL;
+    }
+    hash->entries = (struct hash_entry *)SDL_calloc(1, bytes);
     if (!hash->entries) {
         SDL_free(hash->table);
         SDL_free(hash);