Fix CVE-2016-6207 (denial of service caused by integer overflow in _gdContributionsAlloc()): Copied from upstream commits: From 819ae1b7fce4a61a1492640dd08daa19066af5ab Mon Sep 17 00:00:00 2001 From: Pierre Joye Date: Tue, 19 Jul 2016 14:45:56 +0700 Subject: [PATCH] fix possible OOB or OOM in gdImageScale, reported by Secunia (CVE 2016-6207) --- src/gd.c | 89 +++++++++++------------- src/gd_interpolation.c | 47 +++++++++++-- tests/gdimagescale/CMakeLists.txt | 1 + tests/gdimagescale/Makemodule.am | 3 +- tests/gdimagescale/bug_overflow_large_new_size.c | 31 +++++++++ 5 files changed, 116 insertions(+), 55 deletions(-) create mode 100644 tests/gdimagescale/bug_overflow_large_new_size.c diff --git a/src/gd.c b/src/gd.c index 855e8ca..7faf066 100644 --- a/src/gd.c +++ b/src/gd.c @@ -272,7 +272,7 @@ BGD_DECLARE(gdImagePtr) gdImageCreateTrueColor (int sx, int sy) return 0; } - if (overflow2(sizeof(int), sx)) { + if (overflow2(sizeof(int *), sx)) { return NULL; } @@ -2946,78 +2946,77 @@ BGD_DECLARE(void) gdImageCopyResampled (gdImagePtr dst, int dstW, int dstH, int srcW, int srcH) { int x, y; - double sy1, sy2, sx1, sx2; if (!dst->trueColor) { - gdImageCopyResized (dst, src, dstX, dstY, srcX, srcY, dstW, dstH, - srcW, srcH); + gdImageCopyResized (dst, src, dstX, dstY, srcX, srcY, dstW, dstH, srcW, srcH); return; } for (y = dstY; (y < dstY + dstH); y++) { - sy1 = ((double) y - (double) dstY) * (double) srcH / (double) dstH; - sy2 = ((double) (y + 1) - (double) dstY) * (double) srcH / - (double) dstH; for (x = dstX; (x < dstX + dstW); x++) { - double sx, sy; - double spixels = 0; - double red = 0.0, green = 0.0, blue = 0.0, alpha = 0.0; - double alpha_sum = 0.0, contrib_sum = 0.0; - - sx1 = ((double) x - (double) dstX) * (double) srcW / dstW; - sx2 = ((double) (x + 1) - (double) dstX) * (double) srcW / dstW; + float sy1, sy2, sx1, sx2; + float sx, sy; + float spixels = 0.0; + float red = 0.0, green = 0.0, blue = 0.0, alpha = 0.0; + float alpha_factor, alpha_sum = 0.0, contrib_sum = 0.0; + sy1 = ((float)(y - dstY)) * (float)srcH / (float)dstH; + sy2 = ((float)(y + 1 - dstY)) * (float) srcH / (float) dstH; sy = sy1; do { - double yportion; - if (floor2 (sy) == floor2 (sy1)) { - yportion = 1.0 - (sy - floor2 (sy)); + float yportion; + if (floorf(sy) == floorf(sy1)) { + yportion = 1.0 - (sy - floorf(sy)); if (yportion > sy2 - sy1) { yportion = sy2 - sy1; } - sy = floor2 (sy); - } else if (sy == floor2 (sy2)) { - yportion = sy2 - floor2 (sy2); + sy = floorf(sy); + } else if (sy == floorf(sy2)) { + yportion = sy2 - floorf(sy2); } else { yportion = 1.0; } + sx1 = ((float)(x - dstX)) * (float) srcW / dstW; + sx2 = ((float)(x + 1 - dstX)) * (float) srcW / dstW; sx = sx1; do { - double xportion; - double pcontribution; + float xportion; + float pcontribution; int p; - if (floor2 (sx) == floor2 (sx1)) { - xportion = 1.0 - (sx - floor2 (sx)); + if (floorf(sx) == floorf(sx1)) { + xportion = 1.0 - (sx - floorf(sx)); if (xportion > sx2 - sx1) { xportion = sx2 - sx1; } - sx = floor2 (sx); - } else if (sx == floor2 (sx2)) { - xportion = sx2 - floor2 (sx2); + sx = floorf(sx); + } else if (sx == floorf(sx2)) { + xportion = sx2 - floorf(sx2); } else { xportion = 1.0; } pcontribution = xportion * yportion; - /* 2.08: previously srcX and srcY were ignored. - Andrew Pattison */ - p = gdImageGetTrueColorPixel (src, - (int) sx + srcX, - (int) sy + srcY); - red += gdTrueColorGetRed (p) * pcontribution; - green += gdTrueColorGetGreen (p) * pcontribution; - blue += gdTrueColorGetBlue (p) * pcontribution; + p = gdImageGetTrueColorPixel(src, (int) sx + srcX, (int) sy + srcY); + + alpha_factor = ((gdAlphaMax - gdTrueColorGetAlpha(p))) * pcontribution; + red += gdTrueColorGetRed (p) * alpha_factor; + green += gdTrueColorGetGreen (p) * alpha_factor; + blue += gdTrueColorGetBlue (p) * alpha_factor; alpha += gdTrueColorGetAlpha (p) * pcontribution; + alpha_sum += alpha_factor; + contrib_sum += pcontribution; spixels += xportion * yportion; sx += 1.0; - } while (sx < sx2); - sy += 1.0; - } while (sy < sy2); + } + while (sx < sx2); + sy += 1.0f; + } + while (sy < sy2); + if (spixels != 0.0) { red /= spixels; green /= spixels; blue /= spixels; alpha /= spixels; - alpha += 0.5; } - if ( alpha_sum != 0.0f) { - if( contrib_sum != 0.0f) { + if ( alpha_sum != 0.0) { + if( contrib_sum != 0.0) { alpha_sum /= contrib_sum; } red /= alpha_sum; @@ -3031,17 +3030,13 @@ BGD_DECLARE(void) gdImageCopyResampled (gdImagePtr dst, if (green > 255.0) { green = 255.0; } - if (blue > 255.0) { + if (blue > 255.0f) { blue = 255.0; } if (alpha > gdAlphaMax) { alpha = gdAlphaMax; } - gdImageSetPixel (dst, - x, y, - gdTrueColorAlpha ((int) red, - (int) green, - (int) blue, (int) alpha)); + gdImageSetPixel(dst, x, y, gdTrueColorAlpha ((int) red, (int) green, (int) blue, (int) alpha)); } } } diff --git a/src/gd_interpolation.c b/src/gd_interpolation.c index da6c8ad..72845d2 100644 --- a/src/gd_interpolation.c +++ b/src/gd_interpolation.c @@ -829,6 +829,7 @@ static inline LineContribType * _gdContributionsAlloc(unsigned int line_length, { unsigned int u = 0; LineContribType *res; + int overflow_error = 0; res = (LineContribType *) gdMalloc(sizeof(LineContribType)); if (!res) { @@ -836,10 +837,28 @@ static inline LineContribType * _gdContributionsAlloc(unsigned int line_length, } res->WindowSize = windows_size; res->LineLength = line_length; + if (overflow2(line_length, sizeof(ContributionType))) { + return NULL; + } res->ContribRow = (ContributionType *) gdMalloc(line_length * sizeof(ContributionType)); - + if (res->ContribRow == NULL) { + gdFree(res); + return NULL; + } for (u = 0 ; u < line_length ; u++) { - res->ContribRow[u].Weights = (double *) gdMalloc(windows_size * sizeof(double)); + if (overflow2(windows_size, sizeof(double))) { + overflow_error = 1; + } else { + res->ContribRow[u].Weights = (double *) gdMalloc(windows_size * sizeof(double)); + } + if (overflow_error == 1 || res->ContribRow[u].Weights == NULL) { + u--; + while (u >= 0) { + gdFree(res->ContribRow[u].Weights); + u--; + } + return NULL; + } } return res; } @@ -872,7 +891,9 @@ static inline LineContribType *_gdContributionsCalc(unsigned int line_size, unsi windows_size = 2 * (int)ceil(width_d) + 1; res = _gdContributionsAlloc(line_size, windows_size); - + if (res == NULL) { + return NULL; + } for (u = 0; u < line_size; u++) { const double dCenter = (double)u / scale_d; /* get the significant edge points affecting the pixel */ @@ -977,7 +998,6 @@ _gdScalePass(const gdImagePtr pSrc, const unsigned int src_len, _gdScaleOneAxis(pSrc, pDst, dst_len, line_ndx, contrib, axis); } _gdContributionsFree (contrib); - return 1; }/* _gdScalePass*/ @@ -990,6 +1010,7 @@ gdImageScaleTwoPass(const gdImagePtr src, const unsigned int new_width, const unsigned int src_height = src->sy; gdImagePtr tmp_im = NULL; gdImagePtr dst = NULL; + int scale_pass_res; /* First, handle the trivial case. */ if (src_width == new_width && src_height == new_height) { @@ -1011,7 +1032,11 @@ gdImageScaleTwoPass(const gdImagePtr src, const unsigned int new_width, } gdImageSetInterpolationMethod(tmp_im, src->interpolation_id); - _gdScalePass(src, src_width, tmp_im, new_width, src_height, HORIZONTAL); + scale_pass_res = _gdScalePass(src, src_width, tmp_im, new_width, src_height, HORIZONTAL); + if (scale_pass_res != 1) { + gdImageDestroy(tmp_im); + return NULL; + } }/* if .. else*/ /* If vertical sizes match, we're done. */ @@ -1024,10 +1049,18 @@ gdImageScaleTwoPass(const gdImagePtr src, const unsigned int new_width, dst = gdImageCreateTrueColor(new_width, new_height); if (dst != NULL) { gdImageSetInterpolationMethod(dst, src->interpolation_id); - _gdScalePass(tmp_im, src_height, dst, new_height, new_width, VERTICAL); + scale_pass_res = _gdScalePass(tmp_im, src_height, dst, new_height, new_width, VERTICAL); + if (scale_pass_res != 1) { + gdImageDestroy(dst); + if (src != tmp_im && tmp_im != NULL) { + gdImageDestroy(tmp_im); + } + return NULL; + } }/* if */ - if (src != tmp_im) { + + if (src != tmp_im && tmp_im != NULL) { gdImageDestroy(tmp_im); }/* if */ diff --git a/tests/gdimagescale/CMakeLists.txt b/tests/gdimagescale/CMakeLists.txt index 1098e06..91bd015 100644 --- a/tests/gdimagescale/CMakeLists.txt +++ b/tests/gdimagescale/CMakeLists.txt @@ -1,5 +1,6 @@ SET(TESTS_FILES github_bug_00218 + bug_overflow_large_new_size ) ADD_GD_TESTS() diff --git a/tests/gdimagescale/Makemodule.am b/tests/gdimagescale/Makemodule.am index dacabe7..23b8924 100644 --- a/tests/gdimagescale/Makemodule.am +++ b/tests/gdimagescale/Makemodule.am @@ -1,6 +1,7 @@ libgd_test_programs += \ - gdimagescale/github_bug_00218 + gdimagescale/github_bug_00218 \ + gdimagescale/bug_overflow_large_new_size EXTRA_DIST += \ gdimagescale/CMakeLists.txt diff --git a/tests/gdimagescale/bug_overflow_large_new_size.c b/tests/gdimagescale/bug_overflow_large_new_size.c new file mode 100644 index 0000000..0a8503b --- /dev/null +++ b/tests/gdimagescale/bug_overflow_large_new_size.c @@ -0,0 +1,31 @@ +#include +#include +#include "gd.h" +#include + +#include "gdtest.h" + +int main() +{ + gdImagePtr im, im2; + + im = gdImageCreate(1,1); + if (im == NULL) { + printf("gdImageCreate failed\n"); + return 1; + } + gdImageSetInterpolationMethod(im, GD_BELL); + + /* here the call may pass if the system has enough memory (physical or swap) + or fails (overflow check or alloc fails. + in both cases the tests pass */ + im2 = gdImageScale(im,0x15555556, 1); + if (im2 == NULL) { + printf("gdImageScale failed, expected (out of memory or overflow validation\n"); + return 0; + } + gdImageDestroy(im); + gdImageDestroy(im2); + + return 0; +} -- 2.9.2 From d325888a9fe3c9681e4a9aad576de2c5cd5df2ef Mon Sep 17 00:00:00 2001 From: Pierre Joye Date: Tue, 19 Jul 2016 15:25:47 +0700 Subject: [PATCH 1/3] fix possible OOB or OOM in gdImageScale, reported by Secunia (CVE 2016-6207) --- src/gd_interpolation.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gd_interpolation.c b/src/gd_interpolation.c index 6b7e4ec..602d0f7 100644 --- a/src/gd_interpolation.c +++ b/src/gd_interpolation.c @@ -838,6 +838,7 @@ static inline LineContribType * _gdContributionsAlloc(unsigned int line_length, res->WindowSize = windows_size; res->LineLength = line_length; if (overflow2(line_length, sizeof(ContributionType))) { + gdFree(res); return NULL; } res->ContribRow = (ContributionType *) gdMalloc(line_length * sizeof(ContributionType)); -- 2.9.2 From ff9113c80a32205d45205d3ea30965b25480e0fb Mon Sep 17 00:00:00 2001 From: Pierre Joye Date: Tue, 19 Jul 2016 15:57:08 +0700 Subject: [PATCH 2/3] fix possible OOB or OOM in gdImageScale, reported by Secunia (CVE 2016-6207) --- src/gd_interpolation.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gd_interpolation.c b/src/gd_interpolation.c index 602d0f7..ca1ad10 100644 --- a/src/gd_interpolation.c +++ b/src/gd_interpolation.c @@ -853,11 +853,12 @@ static inline LineContribType * _gdContributionsAlloc(unsigned int line_length, res->ContribRow[u].Weights = (double *) gdMalloc(windows_size * sizeof(double)); } if (overflow_error == 1 || res->ContribRow[u].Weights == NULL) { + unsigned int i; u--; - while (u >= 0) { - gdFree(res->ContribRow[u].Weights); - u--; + for (i=0;i<=u;i++) { + gdFree(res->ContribRow[i].Weights); } + gdFree(res); return NULL; } } -- 2.9.2 From f60ec7a546499f9446063a4dbe755be9523d8232 Mon Sep 17 00:00:00 2001 From: Pierre Joye Date: Tue, 19 Jul 2016 16:30:52 +0700 Subject: [PATCH 3/3] fix possible OOB or OOM in gdImageScale, reported by Secunia (CVE 2016-6207) --- src/gd_interpolation.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gd_interpolation.c b/src/gd_interpolation.c index ca1ad10..c9bcb0c 100644 --- a/src/gd_interpolation.c +++ b/src/gd_interpolation.c @@ -858,6 +858,7 @@ static inline LineContribType * _gdContributionsAlloc(unsigned int line_length, for (i=0;i<=u;i++) { gdFree(res->ContribRow[i].Weights); } + gdFree(res->ContribRow); gdFree(res); return NULL; } -- 2.9.2 From c64a4aeeeeed7b81bc732ca993224ece9ebbc126 Mon Sep 17 00:00:00 2001 From: Pierre Joye Date: Tue, 19 Jul 2016 17:05:54 +0700 Subject: [PATCH] fix possible OOB or OOM in gdImageScale, reported by Secunia (CVE 2016-6207) --- src/gd_interpolation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gd_interpolation.c b/src/gd_interpolation.c index c9bcb0c..3f4b49f 100644 --- a/src/gd_interpolation.c +++ b/src/gd_interpolation.c @@ -1063,7 +1063,7 @@ gdImageScaleTwoPass(const gdImagePtr src, const unsigned int new_width, }/* if */ - if (src != tmp_im && tmp_im != NULL) { + if (tmp_im != NULL && src != tmp_im) { gdImageDestroy(tmp_im); }/* if */ -- 2.9.2