diff --git a/NEWS b/NEWS index 1e406d156f0..67fdd2f098b 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,8 @@ PHP NEWS . ini_set() with mbstring.detect_order changes the order of mb_detect_order as intended, since mbstring.detect_order is an INI_ALL setting. (tobee94) . Added GB18030-2022 to default encoding list for zh-CN. (HeRaNO) + . Fixed bug GH-20836 (Stack overflow in mb_convert_variables with + recursive array references). (alexandre-daubois) - Opcache: . Fixed bug GH-20051 (apache2 shutdowns when restart is requested during diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index ae923469ec3..b2cdd9c7fb8 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -3703,11 +3703,26 @@ next_option: RETVAL_STR(jp_kana_convert(str, enc, opt)); } +static zend_always_inline bool mb_check_stack_limit(void) +{ +#ifdef ZEND_CHECK_STACK_LIMIT + if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) { + zend_call_stack_size_error(); + return true; + } +#endif + return false; +} + static unsigned int mb_recursive_count_strings(zval *var) { unsigned int count = 0; ZVAL_DEREF(var); + if (mb_check_stack_limit()) { + return 0; + } + if (Z_TYPE_P(var) == IS_STRING) { count++; } 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, { ZVAL_DEREF(var); + if (mb_check_stack_limit()) { + return true; + } + if (Z_TYPE_P(var) == IS_STRING) { val_list[*count] = (const unsigned char*)Z_STRVAL_P(var); len_list[*count] = Z_STRLEN_P(var); @@ -3775,6 +3794,10 @@ static bool mb_recursive_convert_variable(zval *var, const mbfl_encoding* from_e { zval *entry, *orig_var; + if (mb_check_stack_limit()) { + return true; + } + orig_var = var; ZVAL_DEREF(var); @@ -3783,17 +3806,25 @@ static bool mb_recursive_convert_variable(zval *var, const mbfl_encoding* from_e zval_ptr_dtor(orig_var); ZVAL_STR(orig_var, ret); } else if (Z_TYPE_P(var) == IS_ARRAY || Z_TYPE_P(var) == IS_OBJECT) { - if (Z_TYPE_P(var) == IS_ARRAY) { - SEPARATE_ARRAY(var); - } - if (Z_REFCOUNTED_P(var)) { - if (Z_IS_RECURSIVE_P(var)) { + HashTable *ht = HASH_OF(var); + HashTable *orig_ht = ht; + + if (ht) { + if (GC_IS_RECURSIVE(ht)) { return true; } - Z_PROTECT_RECURSION_P(var); + + GC_TRY_PROTECT_RECURSION(ht); } - HashTable *ht = HASH_OF(var); + if (Z_TYPE_P(var) == IS_ARRAY) { + SEPARATE_ARRAY(var); + ht = Z_ARRVAL_P(var); + + if (ht && ht != orig_ht && !GC_IS_RECURSIVE(ht)) { + GC_TRY_PROTECT_RECURSION(ht); + } + } if (ht != NULL) { ZEND_HASH_FOREACH_VAL(ht, entry) { /* 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 } if (mb_recursive_convert_variable(entry, from_encoding, to_encoding)) { - if (Z_REFCOUNTED_P(var)) { - Z_UNPROTECT_RECURSION_P(var); + if (ht && ht != orig_ht) { + GC_TRY_UNPROTECT_RECURSION(ht); + } + if (orig_ht) { + GC_TRY_UNPROTECT_RECURSION(orig_ht); } return true; } } ZEND_HASH_FOREACH_END(); } - if (Z_REFCOUNTED_P(var)) { - Z_UNPROTECT_RECURSION_P(var); + if (ht && ht != orig_ht) { + GC_TRY_UNPROTECT_RECURSION(ht); + } + if (orig_ht) { + GC_TRY_UNPROTECT_RECURSION(orig_ht); } } @@ -3895,7 +3932,9 @@ PHP_FUNCTION(mb_convert_variables) efree(ZEND_VOIDP(elist)); efree(ZEND_VOIDP(val_list)); efree(len_list); - php_error_docref(NULL, E_WARNING, "Cannot handle recursive references"); + if (!EG(exception)) { + php_error_docref(NULL, E_WARNING, "Cannot handle recursive references"); + } RETURN_FALSE; } } @@ -3917,7 +3956,9 @@ PHP_FUNCTION(mb_convert_variables) zval *zv = &args[n]; ZVAL_DEREF(zv); if (mb_recursive_convert_variable(zv, from_encoding, to_encoding)) { - php_error_docref(NULL, E_WARNING, "Cannot handle recursive references"); + if (!EG(exception)) { + php_error_docref(NULL, E_WARNING, "Cannot handle recursive references"); + } RETURN_FALSE; } } diff --git a/ext/mbstring/tests/gh20836.phpt b/ext/mbstring/tests/gh20836.phpt new file mode 100644 index 00000000000..60b6a4ce8d4 --- /dev/null +++ b/ext/mbstring/tests/gh20836.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-20836 (Stack overflow in mb_convert_variables with recursive array references) +--EXTENSIONS-- +mbstring +--FILE-- + ['level2' => ['level3' => 'data']]]; +var_dump(mb_convert_variables('utf-8', 'utf-8', $d)); + +echo "Done\n"; +?> +--EXPECTF-- +Warning: mb_convert_variables(): Cannot handle recursive references in %s on line %d +bool(false) + +Warning: mb_convert_variables(): Cannot handle recursive references in %s on line %d +bool(false) +string(5) "UTF-8" +string(5) "UTF-8" +Done diff --git a/ext/mbstring/tests/gh20836_stack_limit.phpt b/ext/mbstring/tests/gh20836_stack_limit.phpt new file mode 100644 index 00000000000..b58833ff80c --- /dev/null +++ b/ext/mbstring/tests/gh20836_stack_limit.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-20836 (Stack overflow in mb_convert_variables with recursive array references, stack limit case) +--EXTENSIONS-- +mbstring +--SKIPIF-- + +--INI-- +zend.max_allowed_stack_size=128K +--FILE-- + createDeepArray($depth - 1)]; +} + +// Create a deeply nested array that will trigger stack limit +$deepArray = createDeepArray(15000); + +mb_convert_variables('utf-8', 'utf-8', $deepArray); + +echo "Done\n"; +?> +--EXPECTF-- +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 +Stack trace: +#0 %s(%d): mb_convert_variables('utf-8', 'utf-8', Array) +#1 {main} + thrown in %s on line %d