Fix double-free in gdImageWebPtr()

The issue is that gdImageWebpCtx() (which is called by gdImageWebpPtr() and
the other WebP output functions to do the real work) does not return whether
it succeeded or failed, so this is not checked in gdImageWebpPtr() and the
function wrongly assumes everything is okay, which is not, in this case,
because there is a size limitation for WebP, namely that the width and
height must by less than 16383.

We can't change the signature of gdImageWebpCtx() for API compatibility
reasons, so we introduce the static helper _gdImageWebpCtx() which returns
success respective failure, so gdImageWebpPtr() and gdImageWebpPtrEx() can
check the return value. We leave it solely to libwebp for now to report
warnings regarding the failing write.

This issue had been reported by Ibrahim El-Sayed to security@libgd.org.

CVE-2016-6912

src/gd_webp.c

@@ -162,37 +162,24 @@ BGD_DECLARE(gdImagePtr) gdImageCreateFromWebpCtx (gdIOCtx * infile)
 	return im;
 }
 
-/*
-  Function: gdImageWebpCtx
 
-    Write the image as WebP data via a <gdIOCtx>. See <gdImageWebpEx>
-    for more details.
-
-  Parameters:
-
-    im      - The image to write.
-    outfile - The output sink.
-    quality - Image quality.
-
-  Returns:
-
-    Nothing.
-*/
-BGD_DECLARE(void) gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
+/* returns 0 on success, 1 on failure */
+static int _gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
 {
 	uint8_t *argb;
 	int x, y;
 	uint8_t *p;
 	uint8_t *out;
 	size_t out_size;
+    int ret = 0;
 
 	if (im == NULL) {
-		return;
+		return 1;
 	}
 
 	if (!gdImageTrueColor(im)) {
-		gd_error("Paletter image not supported by webp");
-		return;
+		gd_error("Palette image not supported by webp");
+		return 1;
 	}
 
 	if (quality == -1) {
@@ -200,16 +187,16 @@ BGD_DECLARE(void) gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
 	}
 
 	if (overflow2(gdImageSX(im), 4)) {
-		return;
+		return 1;
 	}
 
 	if (overflow2(gdImageSX(im) * 4, gdImageSY(im))) {
-		return;
+		return 1;
 	}
 
 	argb = (uint8_t *)gdMalloc(gdImageSX(im) * 4 * gdImageSY(im));
 	if (!argb) {
-		return;
+		return 1;
 	}
 	p = argb;
 	for (y = 0; y < gdImageSY(im); y++) {
@@ -232,6 +219,7 @@ BGD_DECLARE(void) gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
 	out_size = WebPEncodeRGBA(argb, gdImageSX(im), gdImageSY(im), gdImageSX(im) * 4, quality, &out);
 	if (out_size == 0) {
 		gd_error("gd-webp encoding failed");
+        ret = 1;
 		goto freeargb;
 	}
 	gdPutBuf(out, out_size, outfile);
@@ -239,6 +227,30 @@ BGD_DECLARE(void) gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
 
 freeargb:
 	gdFree(argb);
+
+    return ret;
+}
+
+
+/*
+  Function: gdImageWebpCtx
+
+    Write the image as WebP data via a <gdIOCtx>. See <gdImageWebpEx>
+    for more details.
+
+  Parameters:
+
+    im      - The image to write.
+    outfile - The output sink.
+    quality - Image quality.
+
+  Returns:
+
+    Nothing.
+*/
+BGD_DECLARE(void) gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
+{
+	_gdImageWebpCtx(im, outfile, quality);
 }
 
 /*
@@ -278,7 +290,7 @@ BGD_DECLARE(void) gdImageWebpEx (gdImagePtr im, FILE * outFile, int quality)
 	if (out == NULL) {
 		return;
 	}
-	gdImageWebpCtx(im, out, quality);
+	_gdImageWebpCtx(im, out, quality);
 	out->gd_free(out);
 }
 
@@ -302,7 +314,7 @@ BGD_DECLARE(void) gdImageWebp (gdImagePtr im, FILE * outFile)
 	if (out == NULL) {
 		return;
 	}
-	gdImageWebpCtx(im, out, -1);
+	_gdImageWebpCtx(im, out, -1);
 	out->gd_free(out);
 }
 
@@ -318,8 +330,11 @@ BGD_DECLARE(void *) gdImageWebpPtr (gdImagePtr im, int *size)
 	if (out == NULL) {
 		return NULL;
 	}
-	gdImageWebpCtx(im, out, -1);
-	rv = gdDPExtractData(out, size);
+	if (_gdImageWebpCtx(im, out, -1)) {
+		rv = NULL;
+	} else {
+		rv = gdDPExtractData(out, size);
+	}
 	out->gd_free(out);
 
 	return rv;
@@ -337,8 +352,11 @@ BGD_DECLARE(void *) gdImageWebpPtrEx (gdImagePtr im, int *size, int quality)
 	if (out == NULL) {
 		return NULL;
 	}
-	gdImageWebpCtx(im, out, quality);
-	rv = gdDPExtractData(out, size);
+	if (_gdImageWebpCtx(im, out, quality)) {
+        rv = NULL;
+    } else {
+        rv = gdDPExtractData(out, size);
+    }
 	out->gd_free(out);
 	return rv;
 }

tests/webp/.gitignore

@@ -1 +1,2 @@
 /bug00111
+/bug_double_free

tests/webp/CMakeLists.txt

@@ -1,6 +1,7 @@
 IF(WEBP_FOUND)
 LIST(APPEND TESTS_FILES
 	bug00111
+	bug_double_free
 )
 ENDIF(WEBP_FOUND)
 

tests/webp/Makemodule.am

@@ -1,7 +1,9 @@
 if HAVE_LIBWEBP
 libgd_test_programs += \
-	webp/bug00111
+	webp/bug00111 \
+	webp/bug_double_free
 endif
 
 EXTRA_DIST += \
-	webp/CMakeLists.txt
+	webp/CMakeLists.txt \
+	webp/bug_double_free.jpg

tests/webp/bug_double_free.c

@@ -0,0 +1,29 @@
+/**
+ * Test that a too large image doesn't trigger an double-free when written
+ * to memory.
+ */
+
+
+#include "gd.h"
+#include "gdtest.h"
+
+
+int main()
+{
+    gdImagePtr im1, im2;
+    FILE *fp;
+    int size;
+
+    fp = gdTestFileOpen2("webp", "bug_double_free.jpg");
+    gdTestAssert(fp != NULL);
+    im1 = gdImageCreateFromJpeg(fp);
+    gdTestAssert(im1 != NULL);
+    fclose(fp);
+
+    im2 = gdImageWebpPtr(im1, &size);
+    gdTestAssert(im2 == NULL);
+
+    gdImageDestroy(im1);
+
+    return gdNumFailures();
+}