Skip to content

Commit 70f0ca5

Browse files
LamentXU123Girgias
andauthored
Zlib: Improve type checking of several integer options in deflate_init() (#21860)
`zval_get_long()` does a forceful cast and will even cast values like arrays or objects into integers, something that points to a bug. Use the more modern `zval_try_get_long()` API instead. Create a helper function to handle the options consistently. Co-authored-by: Gina Peter Banyard <girgias@php.net>
1 parent 43889fa commit 70f0ca5

4 files changed

Lines changed: 65 additions & 25 deletions

File tree

NEWS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ PHP NEWS
205205

206206
- Zlib:
207207
. deflate_init() now raises a TypeError when the value for option
208-
"strategy" is not of type int. (Weilin Du)
208+
"level", "memory", "window", or "strategy" is not of type int.
209+
(Weilin Du)
209210

210211
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

UPGRADING

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ PHP 8.6 UPGRADE NOTES
103103

104104
- Zlib:
105105
. deflate_init() now raises a TypeError when the value for option
106-
"strategy" is not of type int.
106+
"level", "memory", "window", or "strategy" is not of type int.
107107

108108
========================================
109109
2. New Features
Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,42 @@
11
--TEST--
2-
deflate_init(): strategy option type validation
2+
deflate_init(): options type validation
33
--EXTENSIONS--
44
zlib
55
--FILE--
66
<?php
77

8+
class A {}
9+
$fp = fopen('php://memory', 'r');
10+
811
try {
9-
deflate_init(ZLIB_ENCODING_DEFLATE, ['strategy' => []]);
12+
deflate_init(ZLIB_ENCODING_DEFLATE, ['level' => 'foo']);
1013
} catch (TypeError $e) {
1114
echo $e->getMessage(), PHP_EOL;
1215
}
1316

17+
try {
18+
deflate_init(ZLIB_ENCODING_DEFLATE, ['memory' => []]);
19+
} catch (TypeError $e) {
20+
echo $e->getMessage(), PHP_EOL;
21+
}
22+
23+
try {
24+
deflate_init(ZLIB_ENCODING_DEFLATE, ['window' => new A()]);
25+
} catch (TypeError $e) {
26+
echo $e->getMessage(), PHP_EOL;
27+
}
28+
29+
try {
30+
deflate_init(ZLIB_ENCODING_DEFLATE, ['strategy' => $fp]);
31+
} catch (TypeError $e) {
32+
echo $e->getMessage(), PHP_EOL;
33+
}
34+
35+
fclose($fp);
36+
1437
?>
1538
--EXPECT--
16-
deflate_init(): Argument #2 ($options) the value for option "strategy" must be of type int, array given
39+
deflate_init(): Argument #2 ($options) the value for option "level" must be of type int, string given
40+
deflate_init(): Argument #2 ($options) the value for option "memory" must be of type int, array given
41+
deflate_init(): Argument #2 ($options) the value for option "window" must be of type int, A given
42+
deflate_init(): Argument #2 ($options) the value for option "strategy" must be of type int, resource given

ext/zlib/zlib.c

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,30 @@ static bool zlib_create_dictionary_string(HashTable *options, char **dict, size_
854854
return true;
855855
}
856856

857+
ZEND_ATTRIBUTE_NONNULL static bool zlib_get_long_option(HashTable *options, const char *option_name, size_t option_name_len, zend_long *value)
858+
{
859+
bool failed = false;
860+
zval *option_buffer = zend_hash_str_find(options, option_name, option_name_len);
861+
862+
if (!option_buffer) {
863+
return true;
864+
}
865+
866+
/* The |H ZPP specifier may leave HashTable entries wrapped in IS_INDIRECT. */
867+
ZVAL_DEINDIRECT(option_buffer);
868+
*value = zval_try_get_long(option_buffer, &failed);
869+
if (UNEXPECTED(failed)) {
870+
zend_argument_type_error(
871+
2,
872+
"the value for option \"%.*s\" must be of type int, %s given",
873+
(int) option_name_len, option_name, zend_zval_value_name(option_buffer)
874+
);
875+
return false;
876+
}
877+
878+
return true;
879+
}
880+
857881
/* {{{ Initialize an incremental inflate context with the specified encoding */
858882
PHP_FUNCTION(inflate_init)
859883
{
@@ -1080,49 +1104,38 @@ PHP_FUNCTION(deflate_init)
10801104
zend_long encoding, level = -1, memory = 8, window = 15, strategy = Z_DEFAULT_STRATEGY;
10811105
char *dict = NULL;
10821106
size_t dictlen = 0;
1083-
HashTable *options = NULL;
1084-
zval *option_buffer;
1107+
HashTable *options = (HashTable*)&zend_empty_array;
10851108

10861109
if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS(), "l|H", &encoding, &options)) {
10871110
RETURN_THROWS();
10881111
}
10891112

1090-
if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("level"))) != NULL) {
1091-
ZVAL_DEINDIRECT(option_buffer);
1092-
level = zval_get_long(option_buffer);
1113+
if (!zlib_get_long_option(options, ZEND_STRL("level"), &level)) {
1114+
RETURN_THROWS();
10931115
}
10941116
if (level < -1 || level > 9) {
10951117
zend_value_error("deflate_init(): \"level\" option must be between -1 and 9");
10961118
RETURN_THROWS();
10971119
}
10981120

1099-
if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("memory"))) != NULL) {
1100-
ZVAL_DEINDIRECT(option_buffer);
1101-
memory = zval_get_long(option_buffer);
1121+
if (!zlib_get_long_option(options, ZEND_STRL("memory"), &memory)) {
1122+
RETURN_THROWS();
11021123
}
11031124
if (memory < 1 || memory > 9) {
11041125
zend_value_error("deflate_init(): \"memory\" option must be between 1 and 9");
11051126
RETURN_THROWS();
11061127
}
11071128

1108-
if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("window"))) != NULL) {
1109-
ZVAL_DEINDIRECT(option_buffer);
1110-
window = zval_get_long(option_buffer);
1129+
if (!zlib_get_long_option(options, ZEND_STRL("window"), &window)) {
1130+
RETURN_THROWS();
11111131
}
11121132
if (window < 8 || window > 15) {
11131133
zend_value_error("deflate_init(): \"window\" option must be between 8 and 15");
11141134
RETURN_THROWS();
11151135
}
11161136

1117-
if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("strategy"))) != NULL) {
1118-
bool failed = false;
1119-
1120-
ZVAL_DEINDIRECT(option_buffer);
1121-
strategy = zval_try_get_long(option_buffer, &failed);
1122-
if (UNEXPECTED(failed)) {
1123-
zend_argument_type_error(2, "the value for option \"strategy\" must be of type int, %s given", zend_zval_value_name(option_buffer));
1124-
RETURN_THROWS();
1125-
}
1137+
if (!zlib_get_long_option(options, ZEND_STRL("strategy"), &strategy)) {
1138+
RETURN_THROWS();
11261139
}
11271140
switch (strategy) {
11281141
case Z_FILTERED:

0 commit comments

Comments
 (0)