From f84850767b7ee9f55095c346332849a7ee27a893 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 21 May 2024 19:42:11 +0200 Subject: [PATCH] Fix GH-14290: Member access within null pointer in extension spl php_pcre_replace_impl() can fail and return NULL. We should take that error condition into account. Because other failures return false, we return false here as well. At first, I also thought there was a potential memory leak in the error check of replacement_str, but found that the error condition can never trigger, so replace that with an assertion. --- ext/spl/spl_iterators.c | 11 ++++++++--- ext/spl/tests/gh14290.phpt | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 ext/spl/tests/gh14290.phpt diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index 2f2b800039e39..d546cfd090c8c 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -1904,12 +1904,17 @@ PHP_METHOD(RegexIterator, accept) zval *replacement = zend_read_property(intern->std.ce, Z_OBJ_P(ZEND_THIS), "replacement", sizeof("replacement")-1, 1, &rv); zend_string *replacement_str = zval_try_get_string(replacement); - if (UNEXPECTED(!replacement_str)) { - RETURN_THROWS(); - } + /* Property type is ?string, so this should always succeed. */ + ZEND_ASSERT(replacement_str != NULL); result = php_pcre_replace_impl(intern->u.regex.pce, subject, ZSTR_VAL(subject), ZSTR_LEN(subject), replacement_str, -1, &count); + if (UNEXPECTED(!result)) { + zend_string_release(replacement_str); + zend_string_release_ex(subject, false); + RETURN_FALSE; + } + if (intern->u.regex.flags & REGIT_USE_KEY) { zval_ptr_dtor(&intern->current.key); ZVAL_STR(&intern->current.key, result); diff --git a/ext/spl/tests/gh14290.phpt b/ext/spl/tests/gh14290.phpt new file mode 100644 index 0000000000000..37aba63c80a3c --- /dev/null +++ b/ext/spl/tests/gh14290.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-14290 (Member access within null pointer in extension spl) +--INI-- +pcre.backtrack_limit=2 +pcre.jit=0 +--FILE-- + 'test1']); +$i = new RegexIterator($h, '/^test(.*)/', RegexIterator::REPLACE); +foreach ($i as $name => $value) { + var_dump($name, $value); +} +echo "Done\n"; +?> +--EXPECT-- +Done