From 55f3b360c0f7b44b55bd160c4fbb0d478f7ce52a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 26 May 2019 11:00:20 +0200 Subject: Fix #499: gdImageGifAnimAddPtr: heap corruption with 2 identical images Whenever `gdImageGifAnimAddPtr()` calls `gdImageGifAnimAddCtx()` and the latter fails, we must not call `gdDPExtractData()`; otherwise a double-free would occur. Since `gdImageGifAnimAddCtx` is a void function, and we can't change that for BC reasons, we're introducing a static helper which is used internally. --- src/gd_gif_out.c | 24 ++++++++++++++++++++--- tests/gif/.gitignore | 1 + tests/gif/CMakeLists.txt | 1 + tests/gif/Makemodule.am | 1 + tests/gif/bug00499.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 tests/gif/bug00499.c diff --git a/src/gd_gif_out.c b/src/gd_gif_out.c index d5a9534..7e6726d 100644 --- a/src/gd_gif_out.c +++ b/src/gd_gif_out.c @@ -100,6 +100,10 @@ static void char_out(int c, GifCtx *ctx); static void flush_char(GifCtx *ctx); static int _gdImageGifCtx(gdImagePtr im, gdIOCtxPtr out); +static int _gdImageGifAnimAddCtx(gdImagePtr im, gdIOCtxPtr out, + int LocalCM, int LeftOfs, int TopOfs, + int Delay, int Disposal, + gdImagePtr previm); @@ -487,8 +491,11 @@ BGD_DECLARE(void *) gdImageGifAnimAddPtr(gdImagePtr im, int *size, int LocalCM, void *rv; gdIOCtx *out = gdNewDynamicCtx(2048, NULL); if (out == NULL) return NULL; - gdImageGifAnimAddCtx(im, out, LocalCM, LeftOfs, TopOfs, Delay, Disposal, previm); - rv = gdDPExtractData(out, size); + if (!_gdImageGifAnimAddCtx(im, out, LocalCM, LeftOfs, TopOfs, Delay, Disposal, previm)) { + rv = gdDPExtractData(out, size); + } else { + rv = NULL; + } out->gd_free(out); return rv; } @@ -648,6 +655,15 @@ BGD_DECLARE(void) gdImageGifAnimAddCtx(gdImagePtr im, gdIOCtxPtr out, int LocalCM, int LeftOfs, int TopOfs, int Delay, int Disposal, gdImagePtr previm) +{ + _gdImageGifAnimAddCtx(im, out, LocalCM, LeftOfs, TopOfs, Delay, Disposal, previm); +} + +/* returns 0 on success, 1 on failure */ +static int _gdImageGifAnimAddCtx(gdImagePtr im, gdIOCtxPtr out, + int LocalCM, int LeftOfs, int TopOfs, + int Delay, int Disposal, + gdImagePtr previm) { gdImagePtr pim = NULL, tim = im; int interlace, transparent, BitsPerPixel; @@ -665,7 +681,7 @@ BGD_DECLARE(void) gdImageGifAnimAddCtx(gdImagePtr im, gdIOCtxPtr out, based temporary image. */ pim = gdImageCreatePaletteFromTrueColor(im, 1, 256); if (!pim) { - return; + return 1; } tim = pim; } @@ -838,12 +854,14 @@ break_right: out, tim->sx, tim->sy, LeftOfs, TopOfs, interlace, transparent, Delay, Disposal, BitsPerPixel, LocalCM ? tim->red : 0, tim->green, tim->blue, tim); + return 0; fail_end: if(pim) { /* Destroy palette based temporary image. */ gdImageDestroy(pim); } + return 1; } diff --git a/tests/gif/.gitignore b/tests/gif/.gitignore index 6eb643c..1b07664 100644 --- a/tests/gif/.gitignore +++ b/tests/gif/.gitignore @@ -5,6 +5,7 @@ /bug00066 /bug00181 /bug00227 +/bug00499 /gif_im2im /gif_null /ossfuzz5700 diff --git a/tests/gif/CMakeLists.txt b/tests/gif/CMakeLists.txt index e58e6b0..cb2c940 100644 --- a/tests/gif/CMakeLists.txt +++ b/tests/gif/CMakeLists.txt @@ -2,6 +2,7 @@ LIST(APPEND TESTS_FILES bug00005_2 bug00181 bug00227 + bug00499 gif_null ossfuzz5700 php_bug_75571 diff --git a/tests/gif/Makemodule.am b/tests/gif/Makemodule.am index 5dbeac5..13c7ed7 100644 --- a/tests/gif/Makemodule.am +++ b/tests/gif/Makemodule.am @@ -2,6 +2,7 @@ libgd_test_programs += \ gif/bug00005_2 \ gif/bug00181 \ gif/bug00227 \ + gif/bug00499 \ gif/gif_null \ gif/ossfuzz5700 \ gif/php_bug_75571 \ diff --git a/tests/gif/bug00499.c b/tests/gif/bug00499.c new file mode 100644 index 0000000..0fedb92 --- /dev/null +++ b/tests/gif/bug00499.c @@ -0,0 +1,51 @@ +/** + * Test that adding identical images to GIF animations do no double-free + * + * We are adding two frames to a GIF animation in gdDisposalNone mode, were the + * second frame is identical to the first, which result in that image to have + * zero extent. This program must not cause any memory issues. + * + * See also . + */ + + +#include "gd.h" +#include "gdtest.h" + + +int main() +{ + gdImagePtr im; + int black; + int size; + void * res; + + im = gdImageCreate(100, 100); + black = gdImageColorAllocate(im, 0, 0, 0); + gdImageRectangle(im, 0, 0, 10, 10, black); + + res = gdImageGifAnimBeginPtr(im, &size, 1, 3); + if (res != NULL) { + gdFree(res); + } + + res = gdImageGifAnimAddPtr(im, &size, 0, 0, 0, 100, gdDisposalNone, NULL); + if (res != NULL) { + gdFree(res); + } + + res = gdImageGifAnimAddPtr(im, &size, 0, 0, 0, 100, gdDisposalNone, im); + gdTestAssert(res == NULL); + if (res != NULL) { + gdFree(res); + } + + res = gdImageGifAnimEndPtr(&size); + if (res != NULL) { + gdFree(res); + } + + gdImageDestroy(im); + + return gdNumFailures(); +} -- cgit v1.2.1