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.

5 files changed, 75 insertions, 3 deletions

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;
 }
@@ -649,6 +656,15 @@ BGD_DECLARE(void) gdImageGifAnimAddCtx(gdImagePtr im, gdIOCtxPtr out,
                                        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;
 	interlace = im->interlace;
@@ -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 <https://github.com/libgd/libgd/issues/499>.
+ */
+
+
+#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();
+}