From 2938329ce19cb8c4197dec146c3ec887c6f61d01 Mon Sep 17 00:00:00 2001 From: Xinchen Hui Date: Fri, 27 Dec 2013 14:04:59 +0800 Subject: [PATCH] Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop()) And also fixed the bug: arguments are altered after some calls --- NEWS | 1 + ext/gd/gd.c | 181 ++++++++++++++++++++++++++++++++++++--------- ext/gd/tests/bug66356.phpt | 22 ++++++ main/php_version.h | 6 +- 4 files changed, 173 insertions(+), 37 deletions(-) create mode 100644 ext/gd/tests/bug66356.phpt diff --git a/ext/gd/gd.c b/ext/gd/gd.c index fb25821..49970c1 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -1538,9 +1538,15 @@ PHP_FUNCTION(imagesetstyle) break; } - convert_to_long_ex(item); - - stylearr[index++] = Z_LVAL_PP(item); + if (Z_TYPE_PP(item) != IS_LONG) { + zval lval; + lval = **item; + zval_copy_ctor(&lval); + convert_to_long(&lval); + stylearr[index++] = Z_LVAL(lval); + } else { + stylearr[index++] = Z_LVAL_PP(item); + } } gdImageSetStyle(im, stylearr, index); @@ -3346,14 +3352,26 @@ static void php_imagepolygon(INTERNAL_FUNCTION_PARAMETERS, int filled) for (i = 0; i < npoints; i++) { if (zend_hash_index_find(Z_ARRVAL_P(POINTS), (i * 2), (void **) &var) == SUCCESS) { - SEPARATE_ZVAL((var)); - convert_to_long(*var); - points[i].x = Z_LVAL_PP(var); + if (Z_TYPE_PP(var) != IS_LONG) { + zval lval; + lval = **var; + zval_copy_ctor(&lval); + convert_to_long(&lval); + points[i].x = Z_LVAL(lval); + } else { + points[i].x = Z_LVAL_PP(var); + } } if (zend_hash_index_find(Z_ARRVAL_P(POINTS), (i * 2) + 1, (void **) &var) == SUCCESS) { - SEPARATE_ZVAL(var); - convert_to_long(*var); - points[i].y = Z_LVAL_PP(var); + if (Z_TYPE_PP(var) != IS_LONG) { + zval lval; + lval = **var; + zval_copy_ctor(&lval); + convert_to_long(&lval); + points[i].y = Z_LVAL(lval); + } else { + points[i].y = Z_LVAL_PP(var); + } } } @@ -4859,9 +4877,15 @@ PHP_FUNCTION(imageconvolution) for (j=0; j<3; j++) { if (zend_hash_index_find(Z_ARRVAL_PP(var), (j), (void **) &var2) == SUCCESS) { - SEPARATE_ZVAL(var2); - convert_to_double(*var2); - matrix[i][j] = (float)Z_DVAL_PP(var2); + if (Z_TYPE_PP(var2) != IS_DOUBLE) { + zval dval; + dval = **var; + zval_copy_ctor(&dval); + convert_to_double(&dval); + matrix[i][j] = (float)Z_DVAL(dval); + } else { + matrix[i][j] = (float)Z_DVAL_PP(var2); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "You must have a 3x3 matrix"); RETURN_FALSE; @@ -4954,28 +4978,60 @@ PHP_FUNCTION(imagecrop) ZEND_FETCH_RESOURCE(im, gdImagePtr, &IM, -1, "Image", le_gd); if (zend_hash_find(HASH_OF(z_rect), "x", sizeof("x"), (void **)&tmp) != FAILURE) { - rect.x = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.x = Z_LVAL(lval); + } else { + rect.x = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing x position"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "y", sizeof("x"), (void **)&tmp) != FAILURE) { - rect.y = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.y = Z_LVAL(lval); + } else { + rect.y = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing y position"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "width", sizeof("width"), (void **)&tmp) != FAILURE) { - rect.width = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.width = Z_LVAL(lval); + } else { + rect.width = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing width"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "height", sizeof("height"), (void **)&tmp) != FAILURE) { - rect.height = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.height = Z_LVAL(lval); + } else { + rect.height = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing height"); RETURN_FALSE; @@ -5124,8 +5180,13 @@ PHP_FUNCTION(imageaffine) affine[i] = Z_DVAL_PP(zval_affine_elem); break; case IS_STRING: - convert_to_double_ex(zval_affine_elem); - affine[i] = Z_DVAL_PP(zval_affine_elem); + { + zval dval; + dval = **zval_affine_elem; + zval_copy_ctor(&dval); + convert_to_double(&dval); + affine[i] = Z_DVAL(dval); + } break; default: php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid type for element %i", i); @@ -5136,32 +5197,60 @@ PHP_FUNCTION(imageaffine) if (z_rect != NULL) { if (zend_hash_find(HASH_OF(z_rect), "x", sizeof("x"), (void **)&tmp) != FAILURE) { - convert_to_long_ex(tmp); - rect.x = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.x = Z_LVAL(lval); + } else { + rect.x = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing x position"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "y", sizeof("x"), (void **)&tmp) != FAILURE) { - convert_to_long_ex(tmp); - rect.y = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.y = Z_LVAL(lval); + } else { + rect.y = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing y position"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "width", sizeof("width"), (void **)&tmp) != FAILURE) { - convert_to_long_ex(tmp); - rect.width = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.width = Z_LVAL(lval); + } else { + rect.width = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing width"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(z_rect), "height", sizeof("height"), (void **)&tmp) != FAILURE) { - convert_to_long_ex(tmp); - rect.height = Z_LVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_LONG) { + zval lval; + lval = **tmp; + zval_copy_ctor(&lval); + convert_to_long(&lval); + rect.height = Z_LVAL(lval); + } else { + rect.height = Z_LVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing height"); RETURN_FALSE; @@ -5211,16 +5300,30 @@ PHP_FUNCTION(imageaffinematrixget) php_error_docref(NULL TSRMLS_CC, E_WARNING, "Array expected as options"); } if (zend_hash_find(HASH_OF(options), "x", sizeof("x"), (void **)&tmp) != FAILURE) { - convert_to_double_ex(tmp); - x = Z_DVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_DOUBLE) { + zval dval; + dval = **tmp; + zval_copy_ctor(&dval); + convert_to_double(&dval); + x = Z_DVAL(dval); + } else { + x = Z_DVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing x position"); RETURN_FALSE; } if (zend_hash_find(HASH_OF(options), "y", sizeof("y"), (void **)&tmp) != FAILURE) { - convert_to_double_ex(tmp); - y = Z_DVAL_PP(tmp); + if (Z_TYPE_PP(tmp) != IS_DOUBLE) { + zval dval; + dval = **tmp; + zval_copy_ctor(&dval); + convert_to_double(&dval); + y = Z_DVAL(dval); + } else { + y = Z_DVAL_PP(tmp); + } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing y position"); RETURN_FALSE; @@ -5300,8 +5403,13 @@ PHP_FUNCTION(imageaffinematrixconcat) m1[i] = Z_DVAL_PP(tmp); break; case IS_STRING: - convert_to_double_ex(tmp); - m1[i] = Z_DVAL_PP(tmp); + { + zval dval; + dval = **tmp; + zval_copy_ctor(&dval); + convert_to_double(&dval); + m1[i] = Z_DVAL(dval); + } break; default: php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid type for element %i", i); @@ -5317,8 +5425,13 @@ PHP_FUNCTION(imageaffinematrixconcat) m2[i] = Z_DVAL_PP(tmp); break; case IS_STRING: - convert_to_double_ex(tmp); - m2[i] = Z_DVAL_PP(tmp); + { + zval dval; + dval = **tmp; + zval_copy_ctor(&dval); + convert_to_double(&dval); + m2[i] = Z_DVAL(dval); + } break; default: php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid type for element %i", i); diff --git a/ext/gd/tests/bug66356.phpt b/ext/gd/tests/bug66356.phpt new file mode 100644 index 0000000..f881494 --- /dev/null +++ b/ext/gd/tests/bug66356.phpt @@ -0,0 +1,22 @@ +--TEST-- +Bug #66356 (Heap Overflow Vulnerability in imagecrop()) +--SKIPIF-- + +--FILE-- + "a", "y" => 0, "width" => 10, "height" => 10)); +$arr = array("x" => "a", "y" => "12b", "width" => 10, "height" => 10); +$img = imagecrop($img, $arr); +print_r($arr); +?> +--EXPECTF-- +Array +( + [x] => a + [y] => 12b + [width] => 10 + [height] => 10 +) -- 1.8.4.3 From 8f4a5373bb71590352fd934028d6dde5bc18530b Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Sat, 28 Dec 2013 14:22:13 +0100 Subject: [PATCH] Fixed bug #66356 (Heap Overflow Vulnerability in imagecrop()) Initial fix was PHP stuff This one is libgd fix. - filter invalid crop size - dont try to copy on invalid position - fix crop size when out of src image - fix possible NULL deref - fix possible integer overfloow --- NEWS | 3 ++- ext/gd/libgd/gd_crop.c | 52 ++++++++++++++++++++++++++++------------------ ext/gd/tests/bug66356.phpt | 22 ++++++++++++++++++-- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/ext/gd/libgd/gd_crop.c b/ext/gd/libgd/gd_crop.c index f0b888a..90a99a6 100644 --- a/ext/gd/libgd/gd_crop.c +++ b/ext/gd/libgd/gd_crop.c @@ -44,6 +44,12 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop) { gdImagePtr dst; + /* check size */ + if (crop->width<=0 || crop->height<=0) { + return NULL; + } + + /* allocate the requested size (could be only partially filled) */ if (src->trueColor) { dst = gdImageCreateTrueColor(crop->width, crop->height); gdImageSaveAlpha(dst, 1); @@ -51,37 +57,43 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop) dst = gdImageCreate(crop->width, crop->height); gdImagePaletteCopy(dst, src); } + if (dst == NULL) { + return NULL; + } dst->transparent = src->transparent; - if (src->sx < (crop->x + crop->width -1)) { - crop->width = src->sx - crop->x + 1; + /* check position in the src image */ + if (crop->x < 0 || crop->x>=src->sx || crop->y<0 || crop->y>=src->sy) { + return dst; + } + + /* reduce size if needed */ + if ((src->sx - crop->width) < crop->x) { + crop->width = src->sx - crop->x; } - if (src->sy < (crop->y + crop->height -1)) { - crop->height = src->sy - crop->y + 1; + if ((src->sy - crop->height) < crop->y) { + crop->height = src->sy - crop->y; } + #if 0 printf("rect->x: %i\nrect->y: %i\nrect->width: %i\nrect->height: %i\n", crop->x, crop->y, crop->width, crop->height); #endif - if (dst == NULL) { - return NULL; + int y = crop->y; + if (src->trueColor) { + unsigned int dst_y = 0; + while (y < (crop->y + (crop->height - 1))) { + /* TODO: replace 4 w/byte per channel||pitch once available */ + memcpy(dst->tpixels[dst_y++], src->tpixels[y++] + crop->x, crop->width * 4); + } } else { - int y = crop->y; - if (src->trueColor) { - unsigned int dst_y = 0; - while (y < (crop->y + (crop->height - 1))) { - /* TODO: replace 4 w/byte per channel||pitch once available */ - memcpy(dst->tpixels[dst_y++], src->tpixels[y++] + crop->x, crop->width * 4); - } - } else { - int x; - for (y = crop->y; y < (crop->y + (crop->height - 1)); y++) { - for (x = crop->x; x < (crop->x + (crop->width - 1)); x++) { - dst->pixels[y - crop->y][x - crop->x] = src->pixels[y][x]; - } + int x; + for (y = crop->y; y < (crop->y + (crop->height - 1)); y++) { + for (x = crop->x; x < (crop->x + (crop->width - 1)); x++) { + dst->pixels[y - crop->y][x - crop->x] = src->pixels[y][x]; } } - return dst; } + return dst; } /** diff --git a/ext/gd/tests/bug66356.phpt b/ext/gd/tests/bug66356.phpt index f881494..2da91d6 100644 --- a/ext/gd/tests/bug66356.phpt +++ b/ext/gd/tests/bug66356.phpt @@ -7,12 +7,27 @@ Bug #66356 (Heap Overflow Vulnerability in imagecrop()) --FILE-- "a", "y" => 0, "width" => 10, "height" => 10)); + +// POC #1 +var_dump(imagecrop($img, array("x" => "a", "y" => 0, "width" => 10, "height" => 10))); + $arr = array("x" => "a", "y" => "12b", "width" => 10, "height" => 10); -$img = imagecrop($img, $arr); +var_dump(imagecrop($img, $arr)); print_r($arr); + +// POC #2 +var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => -1, "height" => 10))); + +// POC #3 +var_dump(imagecrop($img, array("x" => -20, "y" => -20, "width" => 10, "height" => 10))); + +// POC #4 +var_dump(imagecrop($img, array("x" => 0x7fffff00, "y" => 0, "width" => 10, "height" => 10))); + ?> --EXPECTF-- +resource(%d) of type (gd) +resource(%d) of type (gd) Array ( [x] => a @@ -20,3 +35,6 @@ Array [width] => 10 [height] => 10 ) +bool(false) +resource(%d) of type (gd) +resource(%d) of type (gd) \ No newline at end of file -- 1.8.4.3 From 464c219ed4ebce6b9196cae308967ac7f7f58bde Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Sat, 28 Dec 2013 14:29:14 +0100 Subject: [PATCH] minor fix on previous --- ext/gd/libgd/gd_crop.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/gd/libgd/gd_crop.c b/ext/gd/libgd/gd_crop.c index 90a99a6..bba425d 100644 --- a/ext/gd/libgd/gd_crop.c +++ b/ext/gd/libgd/gd_crop.c @@ -43,6 +43,7 @@ static int gdColorMatch(gdImagePtr im, int col1, int col2, float threshold); gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop) { gdImagePtr dst; + int y; /* check size */ if (crop->width<=0 || crop->height<=0) { @@ -78,7 +79,7 @@ gdImagePtr gdImageCrop(gdImagePtr src, const gdRectPtr crop) #if 0 printf("rect->x: %i\nrect->y: %i\nrect->width: %i\nrect->height: %i\n", crop->x, crop->y, crop->width, crop->height); #endif - int y = crop->y; + y = crop->y; if (src->trueColor) { unsigned int dst_y = 0; while (y < (crop->y + (crop->height - 1))) { -- 1.8.4.3