Fix a NULL pointer dereference bug lead by php_pcre_replace_impl()

[zhou1615]

php_pcre_replace_impl() will return NULL on failure. However in the function
zim_RegexIterator_accept(), the return value of php_pcre_replace_impl()
is directly used without any check, which could lead to NULL
pointer dereference.

Fix this by adding a NULL check.

This bug is found by a static analyzer, so it is hard to reproduce it.

[cmb69]

Thanks for the new PR! This is definitely an issue that needs to be fixed in PHP-8.0 and up; I'm not quite sure about what should happen in case php_pcre_replace_impl() returns NULL: throw an exception, raise a warning, or just silently fail like it is now in this PR.

Anyhow, a test case matching the PR as is (without the fix, this reproduces the segfault):

--TEST--
NULL pointer dereference in RegexIterator::accept()
--FILE--
<?php
function words() {
    foreach (["f\xC3oo", "bar", "baz"] as $word) {
        yield $word;
    }
}

$it = new RegexIterator(words(), '/a/u', RegexIterator::REPLACE);
var_dump(iterator_to_array($it));
?>
--EXPECT--
array(2) {
  [1]=>
  string(2) "br"
  [2]=>
  string(2) "bz"
}

[Girgias]

Thanks for the new PR! This is definitely an issue that needs to be fixed in PHP-8.0 and up; I'm not quite sure about what should happen in case php_pcre_replace_impl() returns NULL: throw an exception, raise a warning, or just silently fail like it is now in this PR.

Anyhow, a test case matching the PR as is (without the fix, this reproduces the segfault):

--TEST--
NULL pointer dereference in RegexIterator::accept()
--FILE--
<?php
function words() {
    foreach (["f\xC3oo", "bar", "baz"] as $word) {
        yield $word;
    }
}

$it = new RegexIterator(words(), '/a/u', RegexIterator::REPLACE);
var_dump(iterator_to_array($it));
?>
--EXPECT--
array(2) {
  [1]=>
  string(2) "br"
  [2]=>
  string(2) "bz"
}

If this segfaulted before I suppose we can add an exception? As this seems to be some sort of bug

[cmb69]

If this segfaulted before I suppose we can add an exception? As this seems to be some sort of bug

Yes, I think the edge-case of a failing preg_replace() has not been properly handled so far.

[nielsdos]

This fix is incomplete as it leaks memory, but fortunately this was already fixed by b3a56bd (including a test)