Skip to content
Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cmb69 committed Dec 13, 2016
1 parent 4859d69 commit a49feea
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 30 deletions.
74 changes: 46 additions & 28 deletions src/gd_webp.c
Expand Up @@ -162,54 +162,41 @@ 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) {
quality = 80;
}

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++) {
Expand All @@ -232,13 +219,38 @@ 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);
free(out);

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);
}

/*
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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;
Expand All @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions tests/webp/.gitignore
@@ -1 +1,2 @@
/bug00111
/bug_double_free
1 change: 1 addition & 0 deletions tests/webp/CMakeLists.txt
@@ -1,6 +1,7 @@
IF(WEBP_FOUND)
LIST(APPEND TESTS_FILES
bug00111
bug_double_free
)
ENDIF(WEBP_FOUND)

Expand Down
6 changes: 4 additions & 2 deletions 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
29 changes: 29 additions & 0 deletions 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();
}
Binary file added tests/webp/bug_double_free.jpg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit a49feea

Please sign in to comment.