Skip to content

Fix stack overflow in mb_check_encoding() and mb_convert_encoding() #25219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eyupcanakman wants to merge 1 commit into from
Closed

Fix stack overflow in mb_check_encoding() and mb_convert_encoding() #25219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ PHP NEWS
- Curl:
. Fixed curl_reset() retaining CURLOPT_PRIVATE after reset. (lacatoire)

- MBString:
. Fixed stack overflow in mb_check_encoding() and mb_convert_encoding() with
deeply nested arrays. (Eyüp Can Akman)

- Opcache:
. Fixed bug GH-25165 (Tracing JIT loops forever once opcache.jit_buffer_size
is exhausted while compiling side traces). (Ilia Alshanetsky)
Expand Down
45 changes: 33 additions & 12 deletions ext/mbstring/mbstring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2776,6 +2776,17 @@ MBSTRING_API zend_string* php_mb_convert_encoding(const char *input, size_t leng
return php_mb_convert_encoding_ex(input, length, to_encoding, from_encoding);
}

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;
}

MBSTRING_API HashTable *php_mb_convert_encoding_recursive(HashTable *input, const mbfl_encoding *to_encoding, const mbfl_encoding **from_encodings, size_t num_from_encodings)
{
HashTable *output, *chash;
Expand All @@ -2787,6 +2798,10 @@ MBSTRING_API HashTable *php_mb_convert_encoding_recursive(HashTable *input, cons
return NULL;
}

if (mb_check_stack_limit()) {
return NULL;
}

if (GC_IS_RECURSIVE(input)) {
GC_UNPROTECT_RECURSION(input);
php_error_docref(NULL, E_WARNING, "Cannot convert recursively referenced values");
Expand Down Expand Up @@ -2830,6 +2845,13 @@ MBSTRING_API HashTable *php_mb_convert_encoding_recursive(HashTable *input, cons
Z_ARRVAL_P(entry), to_encoding, from_encodings, num_from_encodings);
if (chash) {
ZVAL_ARR(&entry_tmp, chash);
} else if (UNEXPECTED(EG(exception))) {
if (key) {
zend_string_release(key);
}
GC_TRY_UNPROTECT_RECURSION(input);
zend_array_destroy(output);
return NULL;
} else {
ZVAL_EMPTY_ARRAY(&entry_tmp);
}
Expand Down Expand Up @@ -2935,7 +2957,9 @@ PHP_FUNCTION(mb_convert_encoding)
HashTable *tmp;
tmp = php_mb_convert_encoding_recursive(
input_ht, to_encoding, from_encodings, num_from_encodings);
RETVAL_ARR(tmp);
if (tmp) {
RETVAL_ARR(tmp);
}
}

if (free_from_encodings) {
Expand Down Expand Up @@ -3737,17 +3761,6 @@ PHP_FUNCTION(mb_convert_kana)
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;
Expand Down Expand Up @@ -5677,6 +5690,10 @@ static bool php_mb_check_encoding_recursive(HashTable *vars, const mbfl_encoding

(void)(idx); /* Suppress spurious compiler warning that `idx` is not used */

if (mb_check_stack_limit()) {
return false;
}

if (GC_IS_RECURSIVE(vars)) {
php_error_docref(NULL, E_WARNING, "Cannot not handle circular references");
return false;
Expand All @@ -5699,6 +5716,10 @@ static bool php_mb_check_encoding_recursive(HashTable *vars, const mbfl_encoding
break;
case IS_ARRAY:
if (!php_mb_check_encoding_recursive(Z_ARRVAL_P(entry), encoding)) {
if (UNEXPECTED(EG(exception))) {
GC_TRY_UNPROTECT_RECURSION(vars);
return false;
}
valid = false;
break;
}
Expand Down
52 changes: 52 additions & 0 deletions ext/mbstring/tests/mb_encoding_recursive_stack_limit.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
--TEST--
Stack overflow in mb_check_encoding() and mb_convert_encoding() with deeply nested arrays
--EXTENSIONS--
mbstring
--SKIPIF--
<?php
if (ini_get('zend.max_allowed_stack_size') === false) {
die('skip No stack limit support');
}
if (getenv('SKIP_ASAN')) {
die('skip ASAN needs different stack limit setting due to more stack space usage');
}
?>
--INI--
zend.max_allowed_stack_size=256K
--FILE--
<?php
$deep = 'x';
for ($i = 0; $i < 30000; $i++) {
$deep = [$deep];
}

/* Two nested arrays, so the second must not be visited after the first throws. */
foreach ([[$deep], [$deep, $deep]] as $input) {
try {
mb_check_encoding($input, 'UTF-8');
} catch (Error $e) {
echo $e->getMessage(), "\n";
var_dump($e->getPrevious());
}
try {
mb_convert_encoding($input, 'UTF-8', 'ISO-8859-1');
} catch (Error $e) {
echo $e->getMessage(), "\n";
var_dump($e->getPrevious());
}
}

unset($input, $e);
while (is_array($deep)) {
$deep = $deep[0];
}
?>
--EXPECTF--
Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion?
NULL
Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion?
NULL
Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion?
NULL
Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion?
NULL
Loading