From 50b1bfba583b12ceb23ef949567bdd914461e524 Mon Sep 17 00:00:00 2001 From: Fabian Giesen Date: Fri, 1 Mar 2019 19:22:44 -0800 Subject: [PATCH 1/2] stb_image: Fix multiple bugs in GIF decoder. 1. Check not just g->out allocation for failure. 2. If an image descriptor specified a 0-width image, this could be used to produce an out-of-bounds write. 3. Fix memory leak in case an error occurs during decoding. Fixes issue #656. --- stb_image.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/stb_image.h b/stb_image.h index 8b71060..f87f527 100644 --- a/stb_image.h +++ b/stb_image.h @@ -6416,7 +6416,8 @@ static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, i g->out = (stbi_uc *) stbi__malloc(4 * g->w * g->h); g->background = (stbi_uc *) stbi__malloc(4 * g->w * g->h); g->history = (stbi_uc *) stbi__malloc(g->w * g->h); - if (g->out == 0) return stbi__errpuc("outofmem", "Out of memory"); + if (!g->out || !g->background || !g->history) + return stbi__errpuc("outofmem", "Out of memory"); // image is treated as "transparent" at the start - ie, nothing overwrites the current background; // background colour is only used for pixels that are not rendered first frame, after that "background" @@ -6484,6 +6485,13 @@ static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, i g->cur_x = g->start_x; g->cur_y = g->start_y; + // if the width of the specified rectangle is 0, that means + // we may not see *any* pixels or the image is malformed; + // to make sure this is caught, move the current y down to + // max_y (which is what out_gif_code checks). + if (w == 0) + g->cur_y = g->max_y; + g->lflags = stbi__get8(s); if (g->lflags & 0x40) { @@ -6503,7 +6511,7 @@ static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, i return stbi__errpuc("missing color table", "Corrupt GIF"); o = stbi__process_gif_raster(s, g); - if (o == NULL) return NULL; + if (!o) return NULL; // if this was the first frame, pcount = g->w * g->h; @@ -6643,6 +6651,9 @@ static void *stbi__gif_load(stbi__context *s, int *x, int *y, int *comp, int req // can be done for multiple frames. if (req_comp && req_comp != 4) u = stbi__convert_format(u, 4, req_comp, g.w, g.h); + } else if (g.out) { + // if there was an error and we allocated an image buffer, free it! + STBI_FREE(g.out); } // free buffers needed for multiple frame loading; From 6570d6a825e2982a4f1c9ae7173a737eed4dd6f0 Mon Sep 17 00:00:00 2001 From: Fabian Giesen Date: Fri, 1 Mar 2019 19:47:59 -0800 Subject: [PATCH 2/2] stb_image: Make GIF reader validate image size. I must've missed it when I did this for the other image loaders. Either way, combined with the previous checkin, this should fix issue #614 properly. Fixes issue #614. --- stb_image.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/stb_image.h b/stb_image.h index f87f527..6f640e1 100644 --- a/stb_image.h +++ b/stb_image.h @@ -6412,19 +6412,22 @@ static stbi_uc *stbi__gif_load_next(stbi__context *s, stbi__gif *g, int *comp, i // on first frame, any non-written pixels get the background colour (non-transparent) first_frame = 0; if (g->out == 0) { - if (!stbi__gif_header(s, g, comp,0)) return 0; // stbi__g_failure_reason set by stbi__gif_header - g->out = (stbi_uc *) stbi__malloc(4 * g->w * g->h); - g->background = (stbi_uc *) stbi__malloc(4 * g->w * g->h); - g->history = (stbi_uc *) stbi__malloc(g->w * g->h); + if (!stbi__gif_header(s, g, comp,0)) return 0; // stbi__g_failure_reason set by stbi__gif_header + if (!stbi__mad3sizes_valid(4, g->w, g->h, 0)) + return stbi__errpuc("too large", "GIF image is too large"); + pcount = g->w * g->h; + g->out = (stbi_uc *) stbi__malloc(4 * pcount); + g->background = (stbi_uc *) stbi__malloc(4 * pcount); + g->history = (stbi_uc *) stbi__malloc(pcount); if (!g->out || !g->background || !g->history) return stbi__errpuc("outofmem", "Out of memory"); // image is treated as "transparent" at the start - ie, nothing overwrites the current background; // background colour is only used for pixels that are not rendered first frame, after that "background" // color refers to the color that was there the previous frame. - memset( g->out, 0x00, 4 * g->w * g->h ); - memset( g->background, 0x00, 4 * g->w * g->h ); // state of the background (starts transparent) - memset( g->history, 0x00, g->w * g->h ); // pixels that were affected previous frame + memset(g->out, 0x00, 4 * pcount); + memset(g->background, 0x00, 4 * pcount); // state of the background (starts transparent) + memset(g->history, 0x00, pcount); // pixels that were affected previous frame first_frame = 1; } else { // second frame - how do we dispoase of the previous one?