From 585b17706cbf1f553bfc507dd9ccbb8253611c9b Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Tue, 11 Jun 2019 00:15:06 -0700 Subject: [PATCH] Fixed TALOS-2019-0844 - XPM image colorhash parsing Code Execution Vulnerability 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. --- IMG_xpm.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/IMG_xpm.c b/IMG_xpm.c index 0ab6a52c..11a030f0 100644 --- 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);