Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix invalid read in gdImageCreateFromTiffPtr()
tiff_invalid_read.tiff is corrupt, and causes an invalid read in
gdImageCreateFromTiffPtr(), but not in gdImageCreateFromTiff(). The culprit
is dynamicGetbuf(), which doesn't check for out-of-bound reads. In this case,
dynamicGetbuf() is called with a negative dp->pos, but also positive buffer
overflows have to be handled, in which case 0 has to be returned (cf. commit
75e29a9).

Fixing dynamicGetbuf() exhibits that the corrupt TIFF would still create
the image, because the return value of TIFFReadRGBAImage() is not checked.
We do that, and let createFromTiffRgba() fail if TIFFReadRGBAImage() fails.

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

CVE-2016-6911
  • Loading branch information
cmb69 committed Dec 13, 2016
1 parent fb0e0cc commit 4859d69
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 19 deletions.
15 changes: 10 additions & 5 deletions src/gd_io_dp.c
Expand Up @@ -263,6 +263,7 @@ static void dynamicPutchar(struct gdIOCtx *ctx, int a)
appendDynamic(dctx->dp, &b, 1);
}

/* returns the number of bytes actually read; 0 on EOF and error */
static int dynamicGetbuf(gdIOCtxPtr ctx, void *buf, int len)
{
int rlen, remain;
Expand All @@ -272,21 +273,25 @@ static int dynamicGetbuf(gdIOCtxPtr ctx, void *buf, int len)
dctx = (dpIOCtxPtr) ctx;
dp = dctx->dp;

if (dp->pos < 0 || dp->pos >= dp->realSize) {
return 0;
}

remain = dp->logicalSize - dp->pos;
if(remain >= len) {
rlen = len;
} else {
if(remain <= 0) {
/* 2.0.34: EOF is incorrect. We use 0 for
* errors and EOF, just like fileGetbuf,
* which is a simple fread() wrapper.
* TBB. Original bug report: Daniel Cowgill. */
return 0; /* NOT EOF */
return 0;
}

rlen = remain;
}

if (dp->pos + rlen > dp->realSize) {
rlen = dp->realSize - dp->pos;
}

memcpy(buf, (void *) ((char *)dp->data + dp->pos), rlen);
dp->pos += rlen;

Expand Down
29 changes: 16 additions & 13 deletions src/gd_tiff.c
Expand Up @@ -759,6 +759,7 @@ static int createFromTiffRgba(TIFF * tif, gdImagePtr im)
int height = im->sy;
uint32 *buffer;
uint32 rgba;
int success;

/* switch off colour merging on target gd image just while we write out
* content - we want to preserve the alpha data until the user chooses
Expand All @@ -771,26 +772,28 @@ static int createFromTiffRgba(TIFF * tif, gdImagePtr im)
return GD_FAILURE;
}

TIFFReadRGBAImage(tif, width, height, buffer, 0);

for(y = 0; y < height; y++) {
for(x = 0; x < width; x++) {
/* if it doesn't already exist, allocate a new colour,
* else use existing one */
rgba = buffer[(y * width + x)];
a = (0xff - TIFFGetA(rgba)) / 2;
color = gdTrueColorAlpha(TIFFGetR(rgba), TIFFGetG(rgba), TIFFGetB(rgba), a);

/* set pixel colour to this colour */
gdImageSetPixel(im, x, height - y - 1, color);
success = TIFFReadRGBAImage(tif, width, height, buffer, 1);

if (success) {
for(y = 0; y < height; y++) {
for(x = 0; x < width; x++) {
/* if it doesn't already exist, allocate a new colour,
* else use existing one */
rgba = buffer[(y * width + x)];
a = (0xff - TIFFGetA(rgba)) / 2;
color = gdTrueColorAlpha(TIFFGetR(rgba), TIFFGetG(rgba), TIFFGetB(rgba), a);

/* set pixel colour to this colour */
gdImageSetPixel(im, x, height - y - 1, color);
}
}
}

gdFree(buffer);

/* now reset colour merge for alpha blending routines */
gdImageAlphaBlending(im, alphaBlendingFlag);
return GD_SUCCESS;
return success;
}

/*
Expand Down
1 change: 1 addition & 0 deletions tests/tiff/.gitignore
@@ -1,3 +1,4 @@
/tiff_dpi
/tiff_im2im
/tiff_null
/tiff_invalid_read
1 change: 1 addition & 0 deletions tests/tiff/CMakeLists.txt
@@ -1,6 +1,7 @@
IF(TIFF_FOUND)
LIST(APPEND TESTS_FILES
tiff_im2im
tiff_invalid_read
tiff_null
tiff_dpi
)
Expand Down
6 changes: 5 additions & 1 deletion tests/tiff/Makemodule.am
Expand Up @@ -2,8 +2,12 @@ if HAVE_LIBTIFF
libgd_test_programs += \
tiff/tiff_dpi \
tiff/tiff_im2im \
tiff/tiff_invalid_read \
tiff/tiff_null
endif

EXTRA_DIST += \
tiff/CMakeLists.txt
tiff/CMakeLists.txt \
tiff/tiff_invalid_read_1.tiff \
tiff/tiff_invalid_read_2.tiff \
tiff/tiff_invalid_read_3.tiff
61 changes: 61 additions & 0 deletions tests/tiff/tiff_invalid_read.c
@@ -0,0 +1,61 @@
/*
We're testing that reading corrupt TIFF files doesn't cause any memory issues,
and that the operation gracefully fails (i.e. gdImageCreateFromTiffPtr() returns
NULL).
*/

#include "gd.h"
#include "gdtest.h"


static void check_file(char *basename);
static size_t read_test_file(char **buffer, char *basename);


int main()
{
check_file("tiff_invalid_read_1.tiff");
check_file("tiff_invalid_read_2.tiff");
check_file("tiff_invalid_read_3.tiff");

return gdNumFailures();
}


static void check_file(char *basename)
{
gdImagePtr im;
char *buffer;
size_t size;

size = read_test_file(&buffer, basename);
im = gdImageCreateFromTiffPtr(size, (void *) buffer);
gdTestAssert(im == NULL);
free(buffer);
}


static size_t read_test_file(char **buffer, char *basename)
{
char *filename;
FILE *fp;
size_t exp_size, act_size;

filename = gdTestFilePath2("tiff", basename);
fp = fopen(filename, "rb");
gdTestAssert(fp != NULL);

fseek(fp, 0, SEEK_END);
exp_size = ftell(fp);
fseek(fp, 0, SEEK_SET);

*buffer = malloc(exp_size);
gdTestAssert(*buffer != NULL);
act_size = fread(*buffer, sizeof(**buffer), exp_size, fp);
gdTestAssert(act_size == exp_size);

fclose(fp);
free(filename);

return act_size;
}
Binary file added tests/tiff/tiff_invalid_read_1.tiff
Binary file not shown.
Binary file added tests/tiff/tiff_invalid_read_2.tiff
Binary file not shown.
Binary file added tests/tiff/tiff_invalid_read_3.tiff
Binary file not shown.

0 comments on commit 4859d69

Please sign in to comment.