Skip to content

Commit 1806ce9

Browse files
nikicnikic
committed
Add max_depth option to unserialize()
Add a max_depth option to unserialize and an unserialize_max_depth ini setting, which can be used to control the depth limit. The default value is 4096. This option is intended to prevent stack overflows during the unserialization of deeply nested structures. This fixes bug #78549 and addresses oss-fuzz #17581, #17589, #17664, and #17788.
1 parent ce769a9 commit 1806ce9

File tree

10 files changed

+279
-16
lines changed

10 files changed

+279
-16
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ PHP NEWS
1212
. Fixed bug #78579 (mb_decode_numericentity: args number inconsistency).
1313
(cmb)
1414

15+
- Standard:
16+
. Fixed bug #78549 (Stack overflow due to nested serialized input). (Nikita)
17+
1518
19 Sep 2019, PHP 7.4.0RC2
1619

1720
- Core:

‎UPGRADING

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,12 @@ PHP 7.4 UPGRADE NOTES
316316

317317
RFC: https://wiki.php.net/rfc/custom_object_serialization
318318

319+
. A new 'max_depth' option for unserialize(), as well as a
320+
unserialize_max_depth ini setting have been added. These control the
321+
maximum depth of structures permitted during unserialization, and are
322+
intended to prevent stack overflows. The default depth limit is 4096 and
323+
can be disabled by setting unserialize_max_depth=0.
324+
319325
. array_merge() and array_merge_recursive() may now be called without any
320326
arguments, in which case they will return an empty array. This is useful
321327
in conjunction with the spread operator, e.g. array_merge(...$arrays).

‎ext/standard/basic_functions.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3667,6 +3667,7 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
36673667
register_html_constants(INIT_FUNC_ARGS_PASSTHRU);
36683668
register_string_constants(INIT_FUNC_ARGS_PASSTHRU);
36693669

3670+
BASIC_MINIT_SUBMODULE(var)
36703671
BASIC_MINIT_SUBMODULE(file)
36713672
BASIC_MINIT_SUBMODULE(pack)
36723673
BASIC_MINIT_SUBMODULE(browscap)

‎ext/standard/basic_functions.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ typedef struct _php_basic_globals {
235235
#endif
236236

237237
int umask;
238+
zend_long unserialize_max_depth;
238239
} php_basic_globals;
239240

240241
#ifdef ZTS

‎ext/standard/php_var.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "ext/standard/basic_functions.h"
2323
#include "zend_smart_str_public.h"
2424

25+
PHP_MINIT_FUNCTION(var);
2526
PHP_FUNCTION(var_dump);
2627
PHP_FUNCTION(var_export);
2728
PHP_FUNCTION(debug_zval_dump);
@@ -50,6 +51,10 @@ PHPAPI php_unserialize_data_t php_var_unserialize_init(void);
5051
PHPAPI void php_var_unserialize_destroy(php_unserialize_data_t d);
5152
PHPAPI HashTable *php_var_unserialize_get_allowed_classes(php_unserialize_data_t d);
5253
PHPAPI void php_var_unserialize_set_allowed_classes(php_unserialize_data_t d, HashTable *classes);
54+
PHPAPI void php_var_unserialize_set_max_depth(php_unserialize_data_t d, zend_long max_depth);
55+
PHPAPI zend_long php_var_unserialize_get_max_depth(php_unserialize_data_t d);
56+
PHPAPI void php_var_unserialize_set_cur_depth(php_unserialize_data_t d, zend_long cur_depth);
57+
PHPAPI zend_long php_var_unserialize_get_cur_depth(php_unserialize_data_t d);
5358

5459
#define PHP_VAR_SERIALIZE_INIT(d) \
5560
(d) = php_var_serialize_init()

‎ext/standard/tests/serialize/max_depth.phpt

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
--TEST--
2+
Bug #78549: Stack overflow due to nested serialized input
3+
--FILE--
4+
<?php
5+
6+
function create_nested_data($depth, $prefix, $suffix, $inner = 'i:0;') {
7+
return str_repeat($prefix, $depth) . $inner . str_repeat($suffix, $depth);
8+
}
9+
10+
echo "Invalid max_depth:\n";
11+
var_dump(unserialize('i:0;', ['max_depth' => 'foo']));
12+
var_dump(unserialize('i:0;', ['max_depth' => -1]));
13+
14+
echo "Array:\n";
15+
var_dump(unserialize(
16+
create_nested_data(128, 'a:1:{i:0;', '}'),
17+
['max_depth' => 128]
18+
) !== false);
19+
var_dump(unserialize(
20+
create_nested_data(129, 'a:1:{i:0;', '}'),
21+
['max_depth' => 128]
22+
));
23+
24+
echo "Object:\n";
25+
var_dump(unserialize(
26+
create_nested_data(128, 'O:8:"stdClass":1:{i:0;', '}'),
27+
['max_depth' => 128]
28+
) !== false);
29+
var_dump(unserialize(
30+
create_nested_data(129, 'O:8:"stdClass":1:{i:0;', '}'),
31+
['max_depth' => 128]
32+
));
33+
34+
// Default depth is 4096
35+
echo "Default depth:\n";
36+
var_dump(unserialize(create_nested_data(4096, 'a:1:{i:0;', '}')) !== false);
37+
var_dump(unserialize(create_nested_data(4097, 'a:1:{i:0;', '}')));
38+
39+
// Depth can also be adjusted using ini setting
40+
echo "Ini setting:\n";
41+
ini_set("unserialize_max_depth", 128);
42+
var_dump(unserialize(create_nested_data(128, 'a:1:{i:0;', '}')) !== false);
43+
var_dump(unserialize(create_nested_data(129, 'a:1:{i:0;', '}')));
44+
45+
// But an explicitly specified depth still takes precedence
46+
echo "Ini setting overridden:\n";
47+
var_dump(unserialize(
48+
create_nested_data(256, 'a:1:{i:0;', '}'),
49+
['max_depth' => 256]
50+
) !== false);
51+
var_dump(unserialize(
52+
create_nested_data(257, 'a:1:{i:0;', '}'),
53+
['max_depth' => 256]
54+
));
55+
56+
// Reset ini setting to a large value,
57+
// so it's clear that it won't be used in the following.
58+
ini_set("unserialize_max_depth", 4096);
59+
60+
class Test implements Serializable {
61+
public function serialize() {
62+
return '';
63+
}
64+
public function unserialize($str) {
65+
// Should fail, due to combined nesting level
66+
var_dump(unserialize(create_nested_data(129, 'a:1:{i:0;', '}')));
67+
// Should succeeed, below combined nesting level
68+
var_dump(unserialize(create_nested_data(128, 'a:1:{i:0;', '}')) !== false);
69+
}
70+
}
71+
echo "Nested unserialize combined depth limit:\n";
72+
var_dump(is_array(unserialize(
73+
create_nested_data(128, 'a:1:{i:0;', '}', 'C:4:"Test":0:{}'),
74+
['max_depth' => 256]
75+
)));
76+
77+
class Test2 implements Serializable {
78+
public function serialize() {
79+
return '';
80+
}
81+
public function unserialize($str) {
82+
// If depth limit is overridden, the depth should be counted
83+
// from zero again.
84+
var_dump(unserialize(
85+
create_nested_data(257, 'a:1:{i:0;', '}'),
86+
['max_depth' => 256]
87+
));
88+
var_dump(unserialize(
89+
create_nested_data(256, 'a:1:{i:0;', '}'),
90+
['max_depth' => 256]
91+
) !== false);
92+
}
93+
}
94+
echo "Nested unserialize overridden depth limit:\n";
95+
var_dump(is_array(unserialize(
96+
create_nested_data(64, 'a:1:{i:0;', '}', 'C:5:"Test2":0:{}'),
97+
['max_depth' => 128]
98+
)));
99+
100+
?>
101+
--EXPECTF--
102+
Invalid max_depth:
103+
104+
Warning: unserialize(): max_depth should be int in %s on line %d
105+
bool(false)
106+
107+
Warning: unserialize(): max_depth cannot be negative in %s on line %d
108+
bool(false)
109+
Array:
110+
bool(true)
111+
112+
Warning: unserialize(): Maximum depth of 128 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
113+
114+
Notice: unserialize(): Error at offset 1157 of 1294 bytes in %s on line %d
115+
bool(false)
116+
Object:
117+
bool(true)
118+
119+
Warning: unserialize(): Maximum depth of 128 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
120+
121+
Notice: unserialize(): Error at offset 2834 of 2971 bytes in %s on line %d
122+
bool(false)
123+
Default depth:
124+
bool(true)
125+
126+
Warning: unserialize(): Maximum depth of 4096 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
127+
128+
Notice: unserialize(): Error at offset 36869 of 40974 bytes in %s on line %d
129+
bool(false)
130+
Ini setting:
131+
bool(true)
132+
133+
Warning: unserialize(): Maximum depth of 128 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
134+
135+
Notice: unserialize(): Error at offset 1157 of 1294 bytes in %s on line %d
136+
bool(false)
137+
Ini setting overridden:
138+
bool(true)
139+
140+
Warning: unserialize(): Maximum depth of 256 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
141+
142+
Notice: unserialize(): Error at offset 2309 of 2574 bytes in %s on line %d
143+
bool(false)
144+
Nested unserialize combined depth limit:
145+
146+
Warning: unserialize(): Maximum depth of 256 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
147+
148+
Notice: unserialize(): Error at offset 1157 of 1294 bytes in %s on line %d
149+
bool(false)
150+
bool(true)
151+
bool(true)
152+
Nested unserialize overridden depth limit:
153+
154+
Warning: unserialize(): Maximum depth of 256 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %s on line %d
155+
156+
Notice: unserialize(): Error at offset 2309 of 2574 bytes in %s on line %d
157+
bool(false)
158+
bool(true)
159+
bool(true)

‎ext/standard/var.c

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,17 +1174,18 @@ PHP_FUNCTION(serialize)
11741174
}
11751175
/* }}} */
11761176

1177-
/* {{{ proto mixed unserialize(string variable_representation[, array allowed_classes])
1177+
/* {{{ proto mixed unserialize(string variable_representation[, array options])
11781178
Takes a string representation of variable and recreates it */
11791179
PHP_FUNCTION(unserialize)
11801180
{
11811181
char *buf = NULL;
11821182
size_t buf_len;
11831183
const unsigned char *p;
11841184
php_unserialize_data_t var_hash;
1185-
zval *options = NULL, *classes = NULL;
1185+
zval *options = NULL;
11861186
zval *retval;
11871187
HashTable *class_hash = NULL, *prev_class_hash;
1188+
zend_long prev_max_depth, prev_cur_depth;
11881189

11891190
ZEND_PARSE_PARAMETERS_START(1, 2)
11901191
Z_PARAM_STRING(buf, buf_len)
@@ -1200,12 +1201,16 @@ PHP_FUNCTION(unserialize)
12001201
PHP_VAR_UNSERIALIZE_INIT(var_hash);
12011202

12021203
prev_class_hash = php_var_unserialize_get_allowed_classes(var_hash);
1204+
prev_max_depth = php_var_unserialize_get_max_depth(var_hash);
1205+
prev_cur_depth = php_var_unserialize_get_cur_depth(var_hash);
12031206
if (options != NULL) {
1204-
classes = zend_hash_str_find(Z_ARRVAL_P(options), "allowed_classes", sizeof("allowed_classes")-1);
1207+
zval *classes, *max_depth;
1208+
1209+
classes = zend_hash_str_find_deref(Z_ARRVAL_P(options), "allowed_classes", sizeof("allowed_classes")-1);
12051210
if (classes && Z_TYPE_P(classes) != IS_ARRAY && Z_TYPE_P(classes) != IS_TRUE && Z_TYPE_P(classes) != IS_FALSE) {
12061211
php_error_docref(NULL, E_WARNING, "allowed_classes option should be array or boolean");
1207-
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
1208-
RETURN_FALSE;
1212+
RETVAL_FALSE;
1213+
goto cleanup;
12091214
}
12101215

12111216
if(classes && (Z_TYPE_P(classes) == IS_ARRAY || !zend_is_true(classes))) {
@@ -1225,12 +1230,29 @@ PHP_FUNCTION(unserialize)
12251230

12261231
/* Exception during string conversion. */
12271232
if (EG(exception)) {
1228-
zend_hash_destroy(class_hash);
1229-
FREE_HASHTABLE(class_hash);
1230-
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
1233+
goto cleanup;
12311234
}
12321235
}
12331236
php_var_unserialize_set_allowed_classes(var_hash, class_hash);
1237+
1238+
max_depth = zend_hash_str_find_deref(Z_ARRVAL_P(options), "max_depth", sizeof("max_depth") - 1);
1239+
if (max_depth) {
1240+
if (Z_TYPE_P(max_depth) != IS_LONG) {
1241+
php_error_docref(NULL, E_WARNING, "max_depth should be int");
1242+
RETVAL_FALSE;
1243+
goto cleanup;
1244+
}
1245+
if (Z_LVAL_P(max_depth) < 0) {
1246+
php_error_docref(NULL, E_WARNING, "max_depth cannot be negative");
1247+
RETVAL_FALSE;
1248+
goto cleanup;
1249+
}
1250+
1251+
php_var_unserialize_set_max_depth(var_hash, Z_LVAL_P(max_depth));
1252+
/* If the max_depth for a nested unserialize() call has been overridden,
1253+
* start counting from zero again (for the nested call only). */
1254+
php_var_unserialize_set_cur_depth(var_hash, 0);
1255+
}
12341256
}
12351257

12361258
if (BG(unserialize).level > 1) {
@@ -1254,13 +1276,16 @@ PHP_FUNCTION(unserialize)
12541276
gc_check_possible_root(ref);
12551277
}
12561278

1279+
cleanup:
12571280
if (class_hash) {
12581281
zend_hash_destroy(class_hash);
12591282
FREE_HASHTABLE(class_hash);
12601283
}
12611284

1262-
/* Reset to previous allowed_classes in case this is a nested call */
1285+
/* Reset to previous options in case this is a nested call */
12631286
php_var_unserialize_set_allowed_classes(var_hash, prev_class_hash);
1287+
php_var_unserialize_set_max_depth(var_hash, prev_max_depth);
1288+
php_var_unserialize_set_cur_depth(var_hash, prev_cur_depth);
12641289
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
12651290

12661291
/* Per calling convention we must not return a reference here, so unwrap. We're doing this at
@@ -1299,3 +1324,13 @@ PHP_FUNCTION(memory_get_peak_usage) {
12991324
RETURN_LONG(zend_memory_peak_usage(real_usage));
13001325
}
13011326
/* }}} */
1327+
1328+
PHP_INI_BEGIN()
1329+
STD_PHP_INI_ENTRY("unserialize_max_depth", "4096", PHP_INI_ALL, OnUpdateLong, unserialize_max_depth, php_basic_globals, basic_globals)
1330+
PHP_INI_END()
1331+
1332+
PHP_MINIT_FUNCTION(var)
1333+
{
1334+
REGISTER_INI_ENTRIES();
1335+
return SUCCESS;
1336+
}

0 commit comments

Comments
 (0)