From: "Christoph M. Becker" Date: Tue, 2 Aug 2016 12:10:33 +0200 Subject: 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. --- src/gd_io_dp.c | 15 ++++++--- src/gd_tiff.c | 27 +++++++++------- tests/tiff/CMakeLists.txt | 1 + tests/tiff/tiff_invalid_read.c | 61 ++++++++++++++++++++++++++++++++++++ tests/tiff/tiff_invalid_read_1.tiff | Bin 0 -> 3304 bytes tests/tiff/tiff_invalid_read_2.tiff | Bin 0 -> 429 bytes tests/tiff/tiff_invalid_read_3.tiff | Bin 0 -> 428 bytes 7 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 tests/tiff/tiff_invalid_read.c create mode 100644 tests/tiff/tiff_invalid_read_1.tiff create mode 100644 tests/tiff/tiff_invalid_read_2.tiff create mode 100644 tests/tiff/tiff_invalid_read_3.tiff diff --git a/src/gd_io_dp.c b/src/gd_io_dp.c index 228bfa5..eda2eeb 100644 --- a/src/gd_io_dp.c +++ b/src/gd_io_dp.c @@ -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; @@ -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; diff --git a/src/gd_tiff.c b/src/gd_tiff.c index b4f1e63..3f20c5b 100644 --- a/src/gd_tiff.c +++ b/src/gd_tiff.c @@ -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 @@ -771,18 +772,20 @@ 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); + } } } @@ -790,7 +793,7 @@ static int createFromTiffRgba(TIFF * tif, gdImagePtr im) /* now reset colour merge for alpha blending routines */ gdImageAlphaBlending(im, alphaBlendingFlag); - return GD_SUCCESS; + return success; } /* diff --git a/tests/tiff/CMakeLists.txt b/tests/tiff/CMakeLists.txt index 03f630c..81f2303 100644 --- a/tests/tiff/CMakeLists.txt +++ b/tests/tiff/CMakeLists.txt @@ -1,6 +1,7 @@ IF(TIFF_FOUND) LIST(APPEND TESTS_FILES tiff_im2im + tiff_invalid_read tiff_null tiff_dpi ) diff --git a/tests/tiff/tiff_invalid_read.c b/tests/tiff/tiff_invalid_read.c new file mode 100644 index 0000000..bed5389 --- /dev/null +++ b/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; +} diff --git a/tests/tiff/tiff_invalid_read_1.tiff b/tests/tiff/tiff_invalid_read_1.tiff new file mode 100644 index 0000000..e0bfa7b --- /dev/null +++ b/tests/tiff/tiff_invalid_read_1.tiff @@ -0,0 +1,3 @@ +II*H ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿþÿþÿþÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿN  + M– }†›*@ > F ()>† ?VŒRHH +×£ÿÿÿÿ€ázTÿÿÿÿÍÌLÿÿÿÿš™™ÿÿÿÿ€ff&ÿÿÿÿð(\ÿÿÿÿ€ PÿÿÿÿX9Tÿÿÿÿ/mnt/ramfs/test1/small_all/47448692a9a9a022ade1ff6de5bba03283e2db91.svg.tiff.tiff diff --git a/tests/tiff/tiff_invalid_read_2.tiff b/tests/tiff/tiff_invalid_read_2.tiff new file mode 100644 index 0000000..934c3e5 --- /dev/null +++ b/tests/tiff/tiff_invalid_read_2.tiff @@ -0,0 +1,3 @@ +II*€ P8 + M`¢ :Ö ()>P? @HH16dd +×£ÿÿÿÿ€ázTÿÿÿÿÍÌLÿÿÿÿš™™ÿÿÿÿ€ff&ÿÿÿÿ\ð(ÿÿÿÿ€ PÿÿÿÿX9Tÿÿÿÿ/mnt/ramfs/test1/small_all/480d3595731ca9e9a9c92ea03a1f6af27aba1717.gif.tiff diff --git a/tests/tiff/tiff_invalid_read_3.tiff b/tests/tiff/tiff_invalid_read_3.tiff new file mode 100644 index 0000000..20ed871 --- /dev/null +++ b/tests/tiff/tiff_invalid_read_3.tiff @@ -0,0 +1,3 @@ +II*€ P8 + M` ÷; ()>P? @HH33dd +×£ÿÿÿÿ€ázTÿÿÿÿÍÌLÿÿÿÿš™™ÿÿÿÿ€ff&ÿÿÿÿð(\ÿÿÿÿ€ PÿÿÿÿX9Tÿÿÿÿ/mnt/ramfs/test1/sall_all/480d3595731ca9e9a9c92ea03a1f6af27aba1717.gif.tiff