From d8c30cf3f0622f1ec8aedd0deb9852b341365a4f Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 8 Mar 2025 19:24:44 +0000 Subject: [PATCH] ext/gd: calls with array types check strengthening. --- ext/gd/gd.c | 63 +++++++++++++++++++++++--- ext/gd/tests/bug66356.phpt | 21 +++++---- ext/gd/tests/gh18005.phpt | 92 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 ext/gd/tests/gh18005.phpt diff --git a/ext/gd/gd.c b/ext/gd/gd.c index 5822c368e3de2..5690c74fecb90 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -652,7 +652,20 @@ PHP_FUNCTION(imagesetstyle) stylearr = safe_emalloc(sizeof(int), num_styles, 0); ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(styles), item) { - stylearr[index++] = zval_get_long(item); + bool failed = false; + ZVAL_DEREF(item); + zend_long tmp = zval_try_get_long(item, &failed); + if (failed) { + efree(stylearr); + zend_argument_type_error(2, "must only have elements of type int, %s given", zend_zval_type_name(item)); + RETURN_THROWS(); + } + if (ZEND_LONG_EXCEEDS_INT(tmp)) { + efree(stylearr); + zend_argument_type_error(2, "elements must be between %d and %d", INT_MIN, INT_MAX); + RETURN_THROWS(); + } + stylearr[index++] = (int) tmp; } ZEND_HASH_FOREACH_END(); gdImageSetStyle(im, stylearr, index); @@ -3648,7 +3661,20 @@ static void php_image_filter_scatter(INTERNAL_FUNCTION_PARAMETERS) colors = emalloc(num_colors * sizeof(int)); ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(hash_colors), color) { - *(colors + i++) = (int) zval_get_long(color); + bool failed = false; + ZVAL_DEREF(color); + zend_long tmp = zval_try_get_long(color, &failed); + if (failed) { + efree(colors); + zend_argument_value_error(5, "value must be of type int, %s given", zend_zval_type_name(color)); + RETURN_THROWS(); + } + if (tmp < 0 || ZEND_LONG_INT_OVFL(tmp)) { + efree(colors); + zend_argument_value_error(5, "value must be between 0 and %d", INT_MAX); + RETURN_THROWS(); + } + colors[i++] = (int) tmp; } ZEND_HASH_FOREACH_END(); RETVAL_BOOL(gdImageScatterColor(im, (int)scatter_sub, (int)scatter_plus, colors, num_colors)); @@ -3822,6 +3848,23 @@ PHP_FUNCTION(imageantialias) } /* }}} */ +static bool php_gd_zval_try_get_c_int(zval *tmp, const char *field, int *res) { + zend_long r; + bool failed = false; + ZVAL_DEREF(tmp); + r = zval_try_get_long(tmp, &failed); + if (failed) { + zend_argument_value_error(2, "\"%s\" key must be of type int, %s given", field, zend_zval_type_name(tmp)); + return false; + } + if (UNEXPECTED(ZEND_LONG_EXCEEDS_INT(r))) { + zend_argument_value_error(2, "\"%s\" key must be between %d and %d", field, INT_MIN, INT_MAX); + return false; + } + *res = (int)r; + return true; +} + /* {{{ Crop an image using the given coordinates and size, x, y, width and height. */ PHP_FUNCTION(imagecrop) { @@ -3840,28 +3883,36 @@ PHP_FUNCTION(imagecrop) im = php_gd_libgdimageptr_from_zval_p(IM); if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "x", sizeof("x") -1)) != NULL) { - rect.x = zval_get_long(tmp); + if (!php_gd_zval_try_get_c_int(tmp, "x", &rect.x)) { + RETURN_THROWS(); + } } else { zend_argument_value_error(2, "must have an \"x\" key"); RETURN_THROWS(); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "y", sizeof("y") - 1)) != NULL) { - rect.y = zval_get_long(tmp); + if (!php_gd_zval_try_get_c_int(tmp, "y", &rect.y)) { + RETURN_THROWS(); + } } else { zend_argument_value_error(2, "must have a \"y\" key"); RETURN_THROWS(); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "width", sizeof("width") - 1)) != NULL) { - rect.width = zval_get_long(tmp); + if (!php_gd_zval_try_get_c_int(tmp, "width", &rect.width)) { + RETURN_THROWS(); + } } else { zend_argument_value_error(2, "must have a \"width\" key"); RETURN_THROWS(); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "height", sizeof("height") - 1)) != NULL) { - rect.height = zval_get_long(tmp); + if (!php_gd_zval_try_get_c_int(tmp, "height", &rect.height)) { + RETURN_THROWS(); + } } else { zend_argument_value_error(2, "must have a \"height\" key"); RETURN_THROWS(); diff --git a/ext/gd/tests/bug66356.phpt b/ext/gd/tests/bug66356.phpt index b65fb54f57184..616a270115c14 100644 --- a/ext/gd/tests/bug66356.phpt +++ b/ext/gd/tests/bug66356.phpt @@ -7,10 +7,14 @@ gd $img = imagecreatetruecolor(10, 10); // POC #1 -var_dump(imagecrop($img, array("x" => "a", "y" => 0, "width" => 10, "height" => 10))); +var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => 10, "height" => 10))); -$arr = array("x" => "a", "y" => "12b", "width" => 10, "height" => 10); -var_dump(imagecrop($img, $arr)); +$arr = array("x" => 2147483647, "y" => 2147483647, "width" => 10, "height" => 10); +try { + imagecrop($img, $arr); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} print_r($arr); // POC #2 @@ -28,12 +32,11 @@ var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => 65535, "height" => --EXPECTF-- object(GdImage)#2 (0) { } -object(GdImage)#2 (0) { -} +imagecrop(): Argument #2 ($rectangle) overflow with "x" and "width" keys Array ( - [x] => a - [y] => 12b + [x] => 2147483647 + [y] => 2147483647 [width] => 10 [height] => 10 ) @@ -41,9 +44,9 @@ Array Warning: imagecrop(): %cne parameter to a memory allocation multiplication is negative or zero, failing operation gracefully in %s on line %d bool(false) -object(GdImage)#2 (0) { +object(GdImage)#3 (0) { } -object(GdImage)#2 (0) { +object(GdImage)#3 (0) { } Warning: imagecrop(): %croduct of memory allocation multiplication would exceed INT_MAX, failing operation gracefully diff --git a/ext/gd/tests/gh18005.phpt b/ext/gd/tests/gh18005.phpt new file mode 100644 index 0000000000000..6e4568cf5e423 --- /dev/null +++ b/ext/gd/tests/gh18005.phpt @@ -0,0 +1,92 @@ +--TEST-- +GH-18005: imagesetstyle, imagefilter, imagecrop array values type checks +--EXTENSIONS-- +gd +--SKIPIF-- + +--FILE-- +getMessage() . PHP_EOL; +} +try { + imagesetstyle($img, [0, PHP_INT_MIN]); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagefilter($img, IMG_FILTER_SCATTER, 0, 0, [new A()]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagefilter($img, IMG_FILTER_SCATTER, 0, 0, [-1]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => PHP_INT_MIN, "y" => 0, "width" => 0, "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => PHP_INT_MIN, "width" => 0, "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => 0, "width" => PHP_INT_MAX, "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => 0, "width" => 0, "height" => PHP_INT_MAX]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} + +try { + imagecrop($img, ["x" => new A(), "y" => 0, "width" => 0, "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => new A(), "width" => 0, "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => 0, "width" => new A(), "height" => 0]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + imagecrop($img, ["x" => 0, "y" => 0, "width" => 0, "height" => new A()]); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} + +$one = 1; +var_dump(imagecrop($img, ["x" => &$one, "y" => &$one, "width" => &$one, "height" => &$one])); +?> +--EXPECTF-- +imagesetstyle(): Argument #2 ($style) must only have elements of type int, A given +imagesetstyle(): Argument #2 ($style) elements must be between %i and %d +imagefilter(): Argument #5 value must be of type int, A given +imagefilter(): Argument #5 value must be between 0 and 2147483647 +imagecrop(): Argument #2 ($rectangle) "x" key must be between %i and %d +imagecrop(): Argument #2 ($rectangle) "y" key must be between %i and %d +imagecrop(): Argument #2 ($rectangle) "width" key must be between %i and %d +imagecrop(): Argument #2 ($rectangle) "height" key must be between %i and %d +imagecrop(): Argument #2 ($rectangle) "x" key must be of type int, A given +imagecrop(): Argument #2 ($rectangle) "y" key must be of type int, A given +imagecrop(): Argument #2 ($rectangle) "width" key must be of type int, A given +imagecrop(): Argument #2 ($rectangle) "height" key must be of type int, A given +object(GdImage)#2 (0) { +}