Skip to content

Commit e3028e6

Browse files
nielsdosnielsdos
committed
Address review comments
1 parent e4c2545 commit e3028e6

21 files changed

+310
-70
lines changed

‎ext/libxml/image_svg.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,34 @@ static int php_libxml_svg_stream_read(void *context, char *buffer, int len)
3737

3838
/* Sanity check that the input only contains characters valid for a dimension (numbers with units, e.g. 5cm).
3939
* This also protects the user against injecting XSS.
40-
* Only accept [0-9a-zA-Z] */
41-
static bool php_libxml_valid_dimension(const xmlChar *input)
40+
* Only accept [0-9]+[a-zA-Z]* */
41+
static bool php_libxml_parse_dimension(const xmlChar *input, const xmlChar **unit_position)
4242
{
43-
if (*input == '\0') {
43+
if (!(*input >= '0' && *input <= '9')) {
4444
return false;
4545
}
46+
47+
input++;
48+
49+
while (*input) {
50+
if (!(*input >= '0' && *input <= '9')) {
51+
if ((*input >= 'a' && *input <= 'z') || (*input >= 'A' && *input <= 'Z')) {
52+
break;
53+
}
54+
return false;
55+
}
56+
input++;
57+
}
58+
59+
*unit_position = input;
60+
4661
while (*input) {
47-
if (!((*input >= '0' && *input <= '9') || (*input >= 'a' && *input <= 'z') || (*input >= 'A' && *input <= 'Z'))) {
62+
if (!((*input >= 'a' && *input <= 'z') || (*input >= 'A' && *input <= 'Z'))) {
4863
return false;
4964
}
5065
input++;
5166
}
67+
5268
return true;
5369
}
5470

@@ -93,7 +109,10 @@ zend_result php_libxml_svg_image_handle(php_stream *stream, struct php_gfxinfo *
93109

94110
xmlChar *width = xmlTextReaderGetAttribute(reader, BAD_CAST "width");
95111
xmlChar *height = xmlTextReaderGetAttribute(reader, BAD_CAST "height");
96-
if (!width || !height || !php_libxml_valid_dimension(width) || !php_libxml_valid_dimension(height)) {
112+
const xmlChar *width_unit_position, *height_unit_position;
113+
if (!width || !height
114+
|| !php_libxml_parse_dimension(width, &width_unit_position)
115+
|| !php_libxml_parse_dimension(height, &height_unit_position)) {
97116
xmlFree(width);
98117
xmlFree(height);
99118
break;
@@ -102,8 +121,16 @@ zend_result php_libxml_svg_image_handle(php_stream *stream, struct php_gfxinfo *
102121
is_svg = true;
103122
if (result) {
104123
*result = ecalloc(1, sizeof(**result));
105-
(*result)->width_str = zend_string_init((const char *) width, xmlStrlen(width), false);
106-
(*result)->height_str = zend_string_init((const char *) height, xmlStrlen(height), false);
124+
(*result)->width = ZEND_STRTOL((const char *) width, NULL, 10);
125+
(*result)->height = ZEND_STRTOL((const char *) height, NULL, 10);
126+
if (*width_unit_position) {
127+
(*result)->width_unit = zend_string_init((const char*) width_unit_position,
128+
xmlStrlen(width_unit_position), false);
129+
}
130+
if (*height_unit_position) {
131+
(*result)->height_unit = zend_string_init((const char*) height_unit_position,
132+
xmlStrlen(height_unit_position), false);
133+
}
107134
}
108135

109136
xmlFree(width);

‎ext/libxml/tests/image/getimagesize.phpt

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ $inputs = [
1414
<x:svg width='4cm' height="8cm" xmlns:x="http://www.w3.org/2000/svg">
1515
XML,
1616
"<SVG width='1' height=\"1\"/>",
17+
"<SVG width='1px' height=\"1\"/>",
18+
"<SVG width='1' height=\"1mm\"/>",
19+
"<SVG width='1' height=\"cm\"/>",
1720
"<SVG width='' height=\"\"/>",
1821
"<SVG width=\"é\" height='1'/>",
1922
"<foo width='1' height=\"1\"/>",
@@ -28,31 +31,68 @@ foreach ($inputs as $input) {
2831
--EXPECTF--
2932
bool(false)
3033
bool(false)
31-
array(5) {
34+
array(6) {
3235
[0]=>
33-
string(3) "4cm"
36+
int(4)
3437
[1]=>
35-
string(3) "8cm"
38+
int(8)
3639
[2]=>
3740
int(%d)
38-
[3]=>
39-
string(24) "width="4cm" height="8cm""
4041
["mime"]=>
4142
string(13) "image/svg+xml"
43+
["width_unit"]=>
44+
string(2) "cm"
45+
["height_unit"]=>
46+
string(2) "cm"
4247
}
43-
array(5) {
48+
array(7) {
4449
[0]=>
45-
string(1) "1"
50+
int(1)
4651
[1]=>
47-
string(1) "1"
52+
int(1)
4853
[2]=>
4954
int(%d)
5055
[3]=>
5156
string(20) "width="1" height="1""
5257
["mime"]=>
5358
string(13) "image/svg+xml"
59+
["width_unit"]=>
60+
string(2) "px"
61+
["height_unit"]=>
62+
string(2) "px"
63+
}
64+
array(7) {
65+
[0]=>
66+
int(1)
67+
[1]=>
68+
int(1)
69+
[2]=>
70+
int(20)
71+
[3]=>
72+
string(20) "width="1" height="1""
73+
["mime"]=>
74+
string(13) "image/svg+xml"
75+
["width_unit"]=>
76+
string(2) "px"
77+
["height_unit"]=>
78+
string(2) "px"
79+
}
80+
array(6) {
81+
[0]=>
82+
int(1)
83+
[1]=>
84+
int(1)
85+
[2]=>
86+
int(20)
87+
["mime"]=>
88+
string(13) "image/svg+xml"
89+
["width_unit"]=>
90+
string(2) "px"
91+
["height_unit"]=>
92+
string(2) "mm"
5493
}
5594
bool(false)
5695
bool(false)
5796
bool(false)
5897
bool(false)
98+
bool(false)

‎ext/standard/image.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,17 +1543,11 @@ static void php_getimagesize_from_stream(php_stream *stream, char *input, zval *
15431543

15441544
if (result) {
15451545
array_init(return_value);
1546-
if (result->width_str) {
1547-
add_index_str(return_value, 0, result->width_str);
1548-
add_index_str(return_value, 1, result->height_str);
1549-
} else {
1550-
add_index_long(return_value, 0, result->width);
1551-
add_index_long(return_value, 1, result->height);
1552-
}
1546+
add_index_long(return_value, 0, result->width);
1547+
add_index_long(return_value, 1, result->height);
15531548
add_index_long(return_value, 2, itype);
1554-
if (result->width_str) {
1555-
add_index_str(return_value, 3, zend_strpprintf_unchecked(0, "width=\"%S\" height=\"%S\"", result->width_str, result->height_str));
1556-
} else {
1549+
if ((!result->width_unit || zend_string_equals_literal(result->width_unit, "px"))
1550+
&& (!result->height_unit || zend_string_equals_literal(result->height_unit, "px"))) {
15571551
char temp[MAX_LENGTH_OF_LONG * 2 + sizeof("width=\"\" height=\"\"")];
15581552
snprintf(temp, sizeof(temp), "width=\"%d\" height=\"%d\"", result->width, result->height);
15591553
add_index_string(return_value, 3, temp);
@@ -1566,6 +1560,18 @@ static void php_getimagesize_from_stream(php_stream *stream, char *input, zval *
15661560
add_assoc_long(return_value, "channels", result->channels);
15671561
}
15681562
add_assoc_string(return_value, "mime", mime_type ? mime_type : php_image_type_to_mime_type(itype));
1563+
1564+
if (result->width_unit) {
1565+
add_assoc_str(return_value, "width_unit", result->width_unit);
1566+
} else {
1567+
add_assoc_string(return_value, "width_unit", "px");
1568+
}
1569+
if (result->height_unit) {
1570+
add_assoc_str(return_value, "height_unit", result->height_unit);
1571+
} else {
1572+
add_assoc_string(return_value, "height_unit", "px");
1573+
}
1574+
15691575
efree(result);
15701576
} else {
15711577
RETURN_FALSE;

‎ext/standard/php_image.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ PHPAPI bool php_is_image_avif(php_stream *stream);
6262
struct php_gfxinfo {
6363
unsigned int width;
6464
unsigned int height;
65-
zend_string *width_str;
66-
zend_string *height_str;
65+
zend_string *width_unit;
66+
zend_string *height_unit;
6767
unsigned int bits;
6868
unsigned int channels;
6969
};

‎ext/standard/tests/image/bug13213.phpt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ var_dump(GetImageSize(__DIR__.'/bug13213.jpg'));
66
?>
77
--EXPECTF--
88
Warning: getimagesize(): Corrupt JPEG data: 2 extraneous bytes before marker in %s%ebug13213.php on line %d
9-
array(7) {
9+
array(9) {
1010
[0]=>
1111
int(1)
1212
[1]=>
@@ -21,4 +21,8 @@ array(7) {
2121
int(3)
2222
["mime"]=>
2323
string(10) "image/jpeg"
24+
["width_unit"]=>
25+
string(2) "px"
26+
["height_unit"]=>
27+
string(2) "px"
2428
}

‎ext/standard/tests/image/bug70052.phpt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ var_dump(getimagesize(__DIR__ . '/bug70052_2.wbmp'));
77
?>
88
--EXPECT--
99
bool(false)
10-
array(5) {
10+
array(7) {
1111
[0]=>
1212
int(3)
1313
[1]=>
@@ -18,4 +18,8 @@ array(5) {
1818
string(20) "width="3" height="3""
1919
["mime"]=>
2020
string(18) "image/vnd.wap.wbmp"
21+
["width_unit"]=>
22+
string(2) "px"
23+
["height_unit"]=>
24+
string(2) "px"
2125
}

‎ext/standard/tests/image/bug71848.phpt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ var_dump(getimagesize(__DIR__ . '/bug71848.jpg', $info));
66
var_dump(array_keys($info));
77
?>
88
--EXPECT--
9-
array(7) {
9+
array(9) {
1010
[0]=>
1111
int(8)
1212
[1]=>
@@ -21,6 +21,10 @@ array(7) {
2121
int(3)
2222
["mime"]=>
2323
string(10) "image/jpeg"
24+
["width_unit"]=>
25+
string(2) "px"
26+
["height_unit"]=>
27+
string(2) "px"
2428
}
2529
array(2) {
2630
[0]=>

‎ext/standard/tests/image/bug72278.phpt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ var_dump(getimagesize(FILENAME));
88
?>
99
--EXPECTF--
1010
Warning: getimagesize(): Corrupt JPEG data: 3 extraneous bytes before marker in %s%ebug72278.php on line %d
11-
array(7) {
11+
array(9) {
1212
[0]=>
1313
int(300)
1414
[1]=>
@@ -23,4 +23,8 @@ array(7) {
2323
int(3)
2424
["mime"]=>
2525
string(10) "image/jpeg"
26+
["width_unit"]=>
27+
string(2) "px"
28+
["height_unit"]=>
29+
string(2) "px"
2630
}

‎ext/standard/tests/image/bug75708.phpt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ var_dump(getimagesize('fs://bug75708.jpg', $info));
4141

4242
?>
4343
--EXPECT--
44-
array(7) {
44+
array(9) {
4545
[0]=>
4646
int(10)
4747
[1]=>
@@ -56,5 +56,8 @@ array(7) {
5656
int(3)
5757
["mime"]=>
5858
string(10) "image/jpeg"
59+
["width_unit"]=>
60+
string(2) "px"
61+
["height_unit"]=>
62+
string(2) "px"
5963
}
60-

0 commit comments

Comments
 (0)