Skip to content

Commit b391c28

Browse files
Merge branch 'PHP-8.5'
* PHP-8.5: Fix phpGH-20836: Stack overflow in mb_convert_variables with recursive array references (php#20839)
2 parents 3d418eb + 3280368 commit b391c28

File tree

4 files changed

+127
-13
lines changed

4 files changed

+127
-13
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ PHP NEWS
4646
. ini_set() with mbstring.detect_order changes the order of mb_detect_order
4747
as intended, since mbstring.detect_order is an INI_ALL setting. (tobee94)
4848
. Added GB18030-2022 to default encoding list for zh-CN. (HeRaNO)
49+
. Fixed bug GH-20836 (Stack overflow in mb_convert_variables with
50+
recursive array references). (alexandre-daubois)
4951

5052
- Opcache:
5153
. Fixed bug GH-20051 (apache2 shutdowns when restart is requested during

ext/mbstring/mbstring.c

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3703,11 +3703,26 @@ PHP_FUNCTION(mb_convert_kana)
37033703
RETVAL_STR(jp_kana_convert(str, enc, opt));
37043704
}
37053705

3706+
static zend_always_inline bool mb_check_stack_limit(void)
3707+
{
3708+
#ifdef ZEND_CHECK_STACK_LIMIT
3709+
if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) {
3710+
zend_call_stack_size_error();
3711+
return true;
3712+
}
3713+
#endif
3714+
return false;
3715+
}
3716+
37063717
static unsigned int mb_recursive_count_strings(zval *var)
37073718
{
37083719
unsigned int count = 0;
37093720
ZVAL_DEREF(var);
37103721

3722+
if (mb_check_stack_limit()) {
3723+
return 0;
3724+
}
3725+
37113726
if (Z_TYPE_P(var) == IS_STRING) {
37123727
count++;
37133728
} else if (Z_TYPE_P(var) == IS_ARRAY || Z_TYPE_P(var) == IS_OBJECT) {
@@ -3738,6 +3753,10 @@ static bool mb_recursive_find_strings(zval *var, const unsigned char **val_list,
37383753
{
37393754
ZVAL_DEREF(var);
37403755

3756+
if (mb_check_stack_limit()) {
3757+
return true;
3758+
}
3759+
37413760
if (Z_TYPE_P(var) == IS_STRING) {
37423761
val_list[*count] = (const unsigned char*)Z_STRVAL_P(var);
37433762
len_list[*count] = Z_STRLEN_P(var);
@@ -3775,6 +3794,10 @@ static bool mb_recursive_convert_variable(zval *var, const mbfl_encoding* from_e
37753794
{
37763795
zval *entry, *orig_var;
37773796

3797+
if (mb_check_stack_limit()) {
3798+
return true;
3799+
}
3800+
37783801
orig_var = var;
37793802
ZVAL_DEREF(var);
37803803

@@ -3783,17 +3806,25 @@ static bool mb_recursive_convert_variable(zval *var, const mbfl_encoding* from_e
37833806
zval_ptr_dtor(orig_var);
37843807
ZVAL_STR(orig_var, ret);
37853808
} else if (Z_TYPE_P(var) == IS_ARRAY || Z_TYPE_P(var) == IS_OBJECT) {
3786-
if (Z_TYPE_P(var) == IS_ARRAY) {
3787-
SEPARATE_ARRAY(var);
3788-
}
3789-
if (Z_REFCOUNTED_P(var)) {
3790-
if (Z_IS_RECURSIVE_P(var)) {
3809+
HashTable *ht = HASH_OF(var);
3810+
HashTable *orig_ht = ht;
3811+
3812+
if (ht) {
3813+
if (GC_IS_RECURSIVE(ht)) {
37913814
return true;
37923815
}
3793-
Z_PROTECT_RECURSION_P(var);
3816+
3817+
GC_TRY_PROTECT_RECURSION(ht);
37943818
}
37953819

3796-
HashTable *ht = HASH_OF(var);
3820+
if (Z_TYPE_P(var) == IS_ARRAY) {
3821+
SEPARATE_ARRAY(var);
3822+
ht = Z_ARRVAL_P(var);
3823+
3824+
if (ht && ht != orig_ht && !GC_IS_RECURSIVE(ht)) {
3825+
GC_TRY_PROTECT_RECURSION(ht);
3826+
}
3827+
}
37973828
if (ht != NULL) {
37983829
ZEND_HASH_FOREACH_VAL(ht, entry) {
37993830
/* Can be a typed property declaration, in which case we need to remove the reference from the source list.
@@ -3812,16 +3843,22 @@ static bool mb_recursive_convert_variable(zval *var, const mbfl_encoding* from_e
38123843
}
38133844

38143845
if (mb_recursive_convert_variable(entry, from_encoding, to_encoding)) {
3815-
if (Z_REFCOUNTED_P(var)) {
3816-
Z_UNPROTECT_RECURSION_P(var);
3846+
if (ht && ht != orig_ht) {
3847+
GC_TRY_UNPROTECT_RECURSION(ht);
3848+
}
3849+
if (orig_ht) {
3850+
GC_TRY_UNPROTECT_RECURSION(orig_ht);
38173851
}
38183852
return true;
38193853
}
38203854
} ZEND_HASH_FOREACH_END();
38213855
}
38223856

3823-
if (Z_REFCOUNTED_P(var)) {
3824-
Z_UNPROTECT_RECURSION_P(var);
3857+
if (ht && ht != orig_ht) {
3858+
GC_TRY_UNPROTECT_RECURSION(ht);
3859+
}
3860+
if (orig_ht) {
3861+
GC_TRY_UNPROTECT_RECURSION(orig_ht);
38253862
}
38263863
}
38273864

@@ -3895,7 +3932,9 @@ PHP_FUNCTION(mb_convert_variables)
38953932
efree(ZEND_VOIDP(elist));
38963933
efree(ZEND_VOIDP(val_list));
38973934
efree(len_list);
3898-
php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
3935+
if (!EG(exception)) {
3936+
php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
3937+
}
38993938
RETURN_FALSE;
39003939
}
39013940
}
@@ -3917,7 +3956,9 @@ PHP_FUNCTION(mb_convert_variables)
39173956
zval *zv = &args[n];
39183957
ZVAL_DEREF(zv);
39193958
if (mb_recursive_convert_variable(zv, from_encoding, to_encoding)) {
3920-
php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
3959+
if (!EG(exception)) {
3960+
php_error_docref(NULL, E_WARNING, "Cannot handle recursive references");
3961+
}
39213962
RETURN_FALSE;
39223963
}
39233964
}

ext/mbstring/tests/gh20836.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-20836 (Stack overflow in mb_convert_variables with recursive array references)
3+
--EXTENSIONS--
4+
mbstring
5+
--FILE--
6+
<?php
7+
8+
$a = [];
9+
$b = [];
10+
$a[] = $b[] = &$a;
11+
var_dump(mb_convert_variables('utf-8', 'utf-8', $a));
12+
13+
$c = [];
14+
$c[] = &$c;
15+
var_dump(mb_convert_variables('utf-8', 'utf-8', $c));
16+
17+
$normal = ['test', 'array'];
18+
var_dump(mb_convert_variables('utf-8', 'utf-8', $normal));
19+
20+
$d = ['level1' => ['level2' => ['level3' => 'data']]];
21+
var_dump(mb_convert_variables('utf-8', 'utf-8', $d));
22+
23+
echo "Done\n";
24+
?>
25+
--EXPECTF--
26+
Warning: mb_convert_variables(): Cannot handle recursive references in %s on line %d
27+
bool(false)
28+
29+
Warning: mb_convert_variables(): Cannot handle recursive references in %s on line %d
30+
bool(false)
31+
string(5) "UTF-8"
32+
string(5) "UTF-8"
33+
Done
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-20836 (Stack overflow in mb_convert_variables with recursive array references, stack limit case)
3+
--EXTENSIONS--
4+
mbstring
5+
--SKIPIF--
6+
<?php
7+
if (ini_get('zend.max_allowed_stack_size') === false) {
8+
die('skip No stack limit support');
9+
}
10+
if (getenv('SKIP_ASAN')) {
11+
die('skip ASAN needs different stack limit setting due to more stack space usage');
12+
}
13+
?>
14+
--INI--
15+
zend.max_allowed_stack_size=128K
16+
--FILE--
17+
<?php
18+
19+
function createDeepArray($depth) {
20+
if ($depth <= 0) {
21+
return 'deep value';
22+
}
23+
return ['nested' => createDeepArray($depth - 1)];
24+
}
25+
26+
// Create a deeply nested array that will trigger stack limit
27+
$deepArray = createDeepArray(15000);
28+
29+
mb_convert_variables('utf-8', 'utf-8', $deepArray);
30+
31+
echo "Done\n";
32+
?>
33+
--EXPECTF--
34+
Fatal error: Uncaught Error: Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion? in %s:%d
35+
Stack trace:
36+
#0 %s(%d): mb_convert_variables('utf-8', 'utf-8', Array)
37+
#1 {main}
38+
thrown in %s on line %d

0 commit comments

Comments
 (0)