Skip to content

Commit 0bf1882

Browse files
devnexendevnexen
committed
ext/gd: calls with array types check strengthening.
1 parent 19eabc6 commit 0bf1882

File tree

3 files changed

+161
-15
lines changed

3 files changed

+161
-15
lines changed

‎ext/gd/gd.c‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,20 @@ PHP_FUNCTION(imagesetstyle)
640640
stylearr = safe_emalloc(num_styles, sizeof(int), 0);
641641

642642
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(styles), item) {
643-
stylearr[index++] = zval_get_long(item);
643+
bool failed = false;
644+
ZVAL_DEREF(item);
645+
zend_long tmp = zval_try_get_long(item, &failed);
646+
if (failed) {
647+
efree(stylearr);
648+
zend_argument_type_error(2, "must only have elements of type int, %s given", zend_zval_type_name(item));
649+
RETURN_THROWS();
650+
}
651+
if (ZEND_LONG_EXCEEDS_INT(tmp)) {
652+
efree(stylearr);
653+
zend_argument_type_error(2, "elements must be between %d and %d", INT_MIN, INT_MAX);
654+
RETURN_THROWS();
655+
}
656+
stylearr[index++] = (int) tmp;
644657
} ZEND_HASH_FOREACH_END();
645658

646659
gdImageSetStyle(im, stylearr, index);
@@ -3595,7 +3608,20 @@ static void php_image_filter_scatter(INTERNAL_FUNCTION_PARAMETERS)
35953608
colors = safe_emalloc(num_colors, sizeof(int), 0);
35963609

35973610
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(hash_colors), color) {
3598-
*(colors + i++) = (int) zval_get_long(color);
3611+
bool failed = false;
3612+
ZVAL_DEREF(color);
3613+
zend_long tmp = zval_try_get_long(color, &failed);
3614+
if (failed) {
3615+
efree(colors);
3616+
zend_argument_type_error(5, "must be of type int, %s given", zend_zval_type_name(color));
3617+
RETURN_THROWS();
3618+
}
3619+
if (tmp < 0 || ZEND_LONG_INT_OVFL(tmp)) {
3620+
efree(colors);
3621+
zend_argument_value_error(5, "value must be between 0 and %d", INT_MAX);
3622+
RETURN_THROWS();
3623+
}
3624+
colors[i++] = (int) tmp;
35993625
} ZEND_HASH_FOREACH_END();
36003626

36013627
RETVAL_BOOL(gdImageScatterColor(im, (int)scatter_sub, (int)scatter_plus, colors, num_colors));
@@ -3763,6 +3789,23 @@ PHP_FUNCTION(imageantialias)
37633789
}
37643790
/* }}} */
37653791

3792+
static bool php_gd_zval_try_get_c_int(zval *tmp, const char *field, int *res) {
3793+
zend_long r;
3794+
bool failed = false;
3795+
ZVAL_DEREF(tmp);
3796+
r = zval_try_get_long(tmp, &failed);
3797+
if (failed) {
3798+
zend_argument_value_error(2, "\"%s\" key must be of type int, %s given", field, zend_zval_type_name(tmp));
3799+
return false;
3800+
}
3801+
if (UNEXPECTED(ZEND_LONG_EXCEEDS_INT(r))) {
3802+
zend_argument_value_error(2, "\"%s\" key must be between %d and %d", field, INT_MIN, INT_MAX);
3803+
return false;
3804+
}
3805+
*res = (int)r;
3806+
return true;
3807+
}
3808+
37663809
/* {{{ Crop an image using the given coordinates and size, x, y, width and height. */
37673810
PHP_FUNCTION(imagecrop)
37683811
{
@@ -3781,28 +3824,36 @@ PHP_FUNCTION(imagecrop)
37813824
im = php_gd_libgdimageptr_from_zval_p(IM);
37823825

37833826
if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "x", sizeof("x") -1)) != NULL) {
3784-
rect.x = zval_get_long(tmp);
3827+
if (!php_gd_zval_try_get_c_int(tmp, "x", &rect.x)) {
3828+
RETURN_THROWS();
3829+
}
37853830
} else {
37863831
zend_argument_value_error(2, "must have an \"x\" key");
37873832
RETURN_THROWS();
37883833
}
37893834

37903835
if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "y", sizeof("y") - 1)) != NULL) {
3791-
rect.y = zval_get_long(tmp);
3836+
if (!php_gd_zval_try_get_c_int(tmp, "y", &rect.y)) {
3837+
RETURN_THROWS();
3838+
}
37923839
} else {
37933840
zend_argument_value_error(2, "must have a \"y\" key");
37943841
RETURN_THROWS();
37953842
}
37963843

37973844
if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "width", sizeof("width") - 1)) != NULL) {
3798-
rect.width = zval_get_long(tmp);
3845+
if (!php_gd_zval_try_get_c_int(tmp, "width", &rect.width)) {
3846+
RETURN_THROWS();
3847+
}
37993848
} else {
38003849
zend_argument_value_error(2, "must have a \"width\" key");
38013850
RETURN_THROWS();
38023851
}
38033852

38043853
if ((tmp = zend_hash_str_find(Z_ARRVAL_P(z_rect), "height", sizeof("height") - 1)) != NULL) {
3805-
rect.height = zval_get_long(tmp);
3854+
if (!php_gd_zval_try_get_c_int(tmp, "height", &rect.height)) {
3855+
RETURN_THROWS();
3856+
}
38063857
} else {
38073858
zend_argument_value_error(2, "must have a \"height\" key");
38083859
RETURN_THROWS();

‎ext/gd/tests/bug66356.phpt‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ gd
77
$img = imagecreatetruecolor(10, 10);
88

99
// POC #1
10-
var_dump(imagecrop($img, array("x" => "a", "y" => 0, "width" => 10, "height" => 10)));
10+
var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => 10, "height" => 10)));
1111

12-
$arr = array("x" => "a", "y" => "12b", "width" => 10, "height" => 10);
13-
var_dump(imagecrop($img, $arr));
12+
$arr = array("x" => 2147483647, "y" => 2147483647, "width" => 10, "height" => 10);
13+
try {
14+
imagecrop($img, $arr);
15+
} catch (\ValueError $e) {
16+
echo $e->getMessage() . PHP_EOL;
17+
}
1418
print_r($arr);
1519

1620
// POC #2
@@ -28,22 +32,21 @@ var_dump(imagecrop($img, array("x" => 0, "y" => 0, "width" => 65535, "height" =>
2832
--EXPECTF--
2933
object(GdImage)#2 (0) {
3034
}
31-
object(GdImage)#2 (0) {
32-
}
35+
imagecrop(): Argument #2 ($rectangle) overflow with "x" and "width" keys
3336
Array
3437
(
35-
[x] => a
36-
[y] => 12b
38+
[x] => 2147483647
39+
[y] => 2147483647
3740
[width] => 10
3841
[height] => 10
3942
)
4043

4144
Warning: imagecrop(): %cne parameter to a memory allocation multiplication is negative or zero, failing operation gracefully
4245
in %s on line %d
4346
bool(false)
44-
object(GdImage)#2 (0) {
47+
object(GdImage)#3 (0) {
4548
}
46-
object(GdImage)#2 (0) {
49+
object(GdImage)#3 (0) {
4750
}
4851

4952
Warning: imagecrop(): %croduct of memory allocation multiplication would exceed INT_MAX, failing operation gracefully

‎ext/gd/tests/gh18005.phpt‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
--TEST--
2+
GH-18005: imagesetstyle, imagefilter, imagecrop array values type checks
3+
--EXTENSIONS--
4+
gd
5+
--SKIPIF--
6+
<?php
7+
if (PHP_INT_SIZE != 8) die('skip this test is for 64bit platforms only');
8+
?>
9+
--FILE--
10+
<?php
11+
class A {}
12+
$img = imagecreatetruecolor(1, 1);
13+
try {
14+
imagesetstyle($img, [0, new A()]);
15+
} catch (\TypeError $e) {
16+
echo $e->getMessage() . PHP_EOL;
17+
}
18+
try {
19+
imagesetstyle($img, [0, PHP_INT_MIN]);
20+
} catch (\TypeError $e) {
21+
echo $e->getMessage() . PHP_EOL;
22+
}
23+
try {
24+
imagefilter($img, IMG_FILTER_SCATTER, 0, 0, [new A()]);
25+
} catch (\TypeError $e) {
26+
echo $e->getMessage() . PHP_EOL;
27+
}
28+
try {
29+
imagefilter($img, IMG_FILTER_SCATTER, 0, 0, [-1]);
30+
} catch (\ValueError $e) {
31+
echo $e->getMessage() . PHP_EOL;
32+
}
33+
try {
34+
imagecrop($img, ["x" => PHP_INT_MIN, "y" => 0, "width" => 0, "height" => 0]);
35+
} catch (\ValueError $e) {
36+
echo $e->getMessage() . PHP_EOL;
37+
}
38+
try {
39+
imagecrop($img, ["x" => 0, "y" => PHP_INT_MIN, "width" => 0, "height" => 0]);
40+
} catch (\ValueError $e) {
41+
echo $e->getMessage() . PHP_EOL;
42+
}
43+
try {
44+
imagecrop($img, ["x" => 0, "y" => 0, "width" => PHP_INT_MAX, "height" => 0]);
45+
} catch (\ValueError $e) {
46+
echo $e->getMessage() . PHP_EOL;
47+
}
48+
try {
49+
imagecrop($img, ["x" => 0, "y" => 0, "width" => 0, "height" => PHP_INT_MAX]);
50+
} catch (\ValueError $e) {
51+
echo $e->getMessage() . PHP_EOL;
52+
}
53+
54+
try {
55+
imagecrop($img, ["x" => new A(), "y" => 0, "width" => 0, "height" => 0]);
56+
} catch (\ValueError $e) {
57+
echo $e->getMessage() . PHP_EOL;
58+
}
59+
try {
60+
imagecrop($img, ["x" => 0, "y" => new A(), "width" => 0, "height" => 0]);
61+
} catch (\ValueError $e) {
62+
echo $e->getMessage() . PHP_EOL;
63+
}
64+
try {
65+
imagecrop($img, ["x" => 0, "y" => 0, "width" => new A(), "height" => 0]);
66+
} catch (\ValueError $e) {
67+
echo $e->getMessage() . PHP_EOL;
68+
}
69+
try {
70+
imagecrop($img, ["x" => 0, "y" => 0, "width" => 0, "height" => new A()]);
71+
} catch (\ValueError $e) {
72+
echo $e->getMessage() . PHP_EOL;
73+
}
74+
75+
$one = 1;
76+
var_dump(imagecrop($img, ["x" => &$one, "y" => &$one, "width" => &$one, "height" => &$one]));
77+
?>
78+
--EXPECTF--
79+
imagesetstyle(): Argument #2 ($style) must only have elements of type int, A given
80+
imagesetstyle(): Argument #2 ($style) elements must be between %i and %d
81+
imagefilter(): Argument #5 must be of type int, A given
82+
imagefilter(): Argument #5 value must be between 0 and 2147483647
83+
imagecrop(): Argument #2 ($rectangle) "x" key must be between %i and %d
84+
imagecrop(): Argument #2 ($rectangle) "y" key must be between %i and %d
85+
imagecrop(): Argument #2 ($rectangle) "width" key must be between %i and %d
86+
imagecrop(): Argument #2 ($rectangle) "height" key must be between %i and %d
87+
imagecrop(): Argument #2 ($rectangle) "x" key must be of type int, A given
88+
imagecrop(): Argument #2 ($rectangle) "y" key must be of type int, A given
89+
imagecrop(): Argument #2 ($rectangle) "width" key must be of type int, A given
90+
imagecrop(): Argument #2 ($rectangle) "height" key must be of type int, A given
91+
object(GdImage)#2 (0) {
92+
}

0 commit comments

Comments
 (0)