Skip to content

Fix GH-25413: Preserve frameless string predicate arguments during coercion #25430

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
rafamolina1 wants to merge 1 commit into from
+266 −12
Closed

Fix GH-25413: Preserve frameless string predicate arguments during coercion #25430

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
2 changes: 2 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,8 @@ PHP NEWS
properties). (Lazizbek Ergashev)

- Standard:
. Fixed bug GH-25413 (Frameless string predicates read arguments changed or
freed during coercion).
. Fixed memory leaks in checkdnsrr() and dns_check_record() on Windows.
(Ilia Alshanetsky)
. Fixed __unserialize() and __wakeup() being called for objects with a missing
Expand Down
43 changes: 31 additions & 12 deletions ext/standard/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1876,19 +1876,41 @@ PHP_FUNCTION(str_contains)
}
/* }}} */

ZEND_FRAMELESS_FUNCTION(str_contains, 2)
static zend_never_inline ZEND_COLD void php_string_predicate_slow(
zval *return_value, zval *arg1, zval *arg2, zend_frameless_function_2 handler)
{
zval haystack_tmp, needle_tmp;
zval haystack_tmp, needle_tmp, haystack_copy, needle_copy;
zend_string *haystack, *needle;

/* Coercion may run user code that changes or frees either argument. */
ZVAL_COPY_DEREF(&haystack_copy, arg1);
ZVAL_COPY_DEREF(&needle_copy, arg2);
arg1 = &haystack_copy;
arg2 = &needle_copy;

Z_FLF_PARAM_STR(1, haystack, haystack_tmp);
Z_FLF_PARAM_STR(2, needle, needle_tmp);

RETVAL_BOOL(php_memnstr(ZSTR_VAL(haystack), ZSTR_VAL(needle), ZSTR_LEN(needle), ZSTR_VAL(haystack) + ZSTR_LEN(haystack)));
/* Both arguments are strings now, so the handler takes its fast path. */
handler(return_value, arg1, arg2);

flf_clean:
Z_FLF_PARAM_FREE_STR(1, haystack_tmp);
Z_FLF_PARAM_FREE_STR(2, needle_tmp);
zval_ptr_dtor(&haystack_copy);
zval_ptr_dtor(&needle_copy);
}

ZEND_FRAMELESS_FUNCTION(str_contains, 2)
{
if (UNEXPECTED(Z_TYPE_P(arg1) != IS_STRING || Z_TYPE_P(arg2) != IS_STRING)) {
php_string_predicate_slow(return_value, arg1, arg2, ZEND_FRAMELESS_FUNCTION_NAME(str_contains, 2));
return;
}

zend_string *haystack = Z_STR_P(arg1);
zend_string *needle = Z_STR_P(arg2);
RETVAL_BOOL(php_memnstr(ZSTR_VAL(haystack), ZSTR_VAL(needle), ZSTR_LEN(needle), ZSTR_VAL(haystack) + ZSTR_LEN(haystack)));
}

/* {{{ Checks if haystack starts with needle */
Expand All @@ -1907,17 +1929,14 @@ PHP_FUNCTION(str_starts_with)

ZEND_FRAMELESS_FUNCTION(str_starts_with, 2)
{
zval haystack_tmp, needle_tmp;
zend_string *haystack, *needle;

Z_FLF_PARAM_STR(1, haystack, haystack_tmp);
Z_FLF_PARAM_STR(2, needle, needle_tmp);
if (UNEXPECTED(Z_TYPE_P(arg1) != IS_STRING || Z_TYPE_P(arg2) != IS_STRING)) {
php_string_predicate_slow(return_value, arg1, arg2, ZEND_FRAMELESS_FUNCTION_NAME(str_starts_with, 2));
return;
}

zend_string *haystack = Z_STR_P(arg1);
zend_string *needle = Z_STR_P(arg2);
RETVAL_BOOL(zend_string_starts_with(haystack, needle));

flf_clean:
Z_FLF_PARAM_FREE_STR(1, haystack_tmp);
Z_FLF_PARAM_FREE_STR(2, needle_tmp);
}

/* {{{ Checks if haystack ends with needle */
Expand Down
178 changes: 178 additions & 0 deletions ext/standard/tests/strings/gh25413.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
--TEST--
GH-25413 (Frameless string predicates must preserve arguments before coercion)
--FILE--
<?php
class CoercePredicateArgument implements Stringable {
public function __construct(private string $action) {}

public function __toString(): string {
switch ($this->action) {
case 'reference needle':
global $needle;
$needle = $needle;
break;
case 'replace needle':
$GLOBALS['needle'] = 'xyz';
break;
case 'unset needle':
case 'unset referenced needle':
unset($GLOBALS['needle']);
break;
case 'modify haystack':
$GLOBALS['haystack'][0] = '!';
break;
case 'unset haystack':
unset($GLOBALS['haystack']);
break;
case 'throw':
throw new Exception('conversion failed');
}
return 'abc';
}
}

$actions = [
'reference needle',
'replace needle',
'unset needle',
'unset referenced needle',
'modify haystack',
'unset haystack',
'throw first',
'throw second',
'invalid first',
'invalid second',
'strings',
'empty needle',
'empty haystack',
'different strings',
'scalar coercion',
];

// Repeat the direct calls so the same cases can also exercise the tracing JIT.
for ($iteration = 0; $iteration < 20; $iteration++) {
foreach (['str_contains', 'str_starts_with', 'str_ends_with'] as $function) {
foreach ($actions as $action) {
$results = [];
foreach ([false, true] as $dynamic) {
unset($haystack, $needle);
$haystack = str_repeat('abc', random_int(1, 1));
$needle = 'abc';
switch ($action) {
case 'reference needle':
case 'replace needle':
case 'unset needle':
$haystack = new CoercePredicateArgument($action);
break;
case 'unset referenced needle':
$alias =& $needle;
unset($alias);
$haystack = new CoercePredicateArgument($action);
break;
case 'modify haystack':
case 'unset haystack':
$needle = new CoercePredicateArgument($action);
break;
case 'throw first':
$haystack = new CoercePredicateArgument('throw');
break;
case 'throw second':
$needle = new CoercePredicateArgument('throw');
break;
case 'invalid first':
$haystack = [];
break;
case 'invalid second':
$haystack = new CoercePredicateArgument('none');
$needle = [];
break;
case 'empty needle':
$needle = '';
break;
case 'empty haystack':
$haystack = '';
break;
case 'different strings':
$needle = 'xyz';
break;
case 'scalar coercion':
$haystack = 123;
$needle = 123;
break;
}

try {
if ($dynamic) {
$result = $function($haystack, $needle);
} else {
$result = match ($function) {
'str_contains' => str_contains($haystack, $needle),
'str_starts_with' => str_starts_with($haystack, $needle),
'str_ends_with' => str_ends_with($haystack, $needle),
};
}
} catch (Throwable $e) {
$result = $e::class . ': ' . $e->getMessage();
}
$results[] = [$result, isset($haystack), isset($needle),
isset($haystack) && is_string($haystack) ? $haystack : null,
isset($needle) && is_string($needle) ? $needle : null];
}
if ($results[0] !== $results[1] && $iteration == 19) {
echo "$function / $action: mismatch\n";
var_dump($results);
}
if ($iteration == 19) {
echo "$function / $action: ";
var_dump($results[0][0]);
}
}
}
}
?>
--EXPECT--
str_contains / reference needle: bool(true)
str_contains / replace needle: bool(true)
str_contains / unset needle: bool(true)
str_contains / unset referenced needle: bool(true)
str_contains / modify haystack: bool(true)
str_contains / unset haystack: bool(true)
str_contains / throw first: string(28) "Exception: conversion failed"
str_contains / throw second: string(28) "Exception: conversion failed"
str_contains / invalid first: string(86) "TypeError: str_contains(): Argument #1 ($haystack) must be of type string, array given"
str_contains / invalid second: string(84) "TypeError: str_contains(): Argument #2 ($needle) must be of type string, array given"
str_contains / strings: bool(true)
str_contains / empty needle: bool(true)
str_contains / empty haystack: bool(false)
str_contains / different strings: bool(false)
str_contains / scalar coercion: bool(true)
str_starts_with / reference needle: bool(true)
str_starts_with / replace needle: bool(true)
str_starts_with / unset needle: bool(true)
str_starts_with / unset referenced needle: bool(true)
str_starts_with / modify haystack: bool(true)
str_starts_with / unset haystack: bool(true)
str_starts_with / throw first: string(28) "Exception: conversion failed"
str_starts_with / throw second: string(28) "Exception: conversion failed"
str_starts_with / invalid first: string(89) "TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, array given"
str_starts_with / invalid second: string(87) "TypeError: str_starts_with(): Argument #2 ($needle) must be of type string, array given"
str_starts_with / strings: bool(true)
str_starts_with / empty needle: bool(true)
str_starts_with / empty haystack: bool(false)
str_starts_with / different strings: bool(false)
str_starts_with / scalar coercion: bool(true)
str_ends_with / reference needle: bool(true)
str_ends_with / replace needle: bool(true)
str_ends_with / unset needle: bool(true)
str_ends_with / unset referenced needle: bool(true)
str_ends_with / modify haystack: bool(true)
str_ends_with / unset haystack: bool(true)
str_ends_with / throw first: string(28) "Exception: conversion failed"
str_ends_with / throw second: string(28) "Exception: conversion failed"
str_ends_with / invalid first: string(87) "TypeError: str_ends_with(): Argument #1 ($haystack) must be of type string, array given"
str_ends_with / invalid second: string(85) "TypeError: str_ends_with(): Argument #2 ($needle) must be of type string, array given"
str_ends_with / strings: bool(true)
str_ends_with / empty needle: bool(true)
str_ends_with / empty haystack: bool(false)
str_ends_with / different strings: bool(false)
str_ends_with / scalar coercion: bool(true)
55 changes: 55 additions & 0 deletions ext/standard/tests/strings/gh25413_strict.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
--TEST--
GH-25413 (Frameless string predicates still reject coercion in strict mode)
--FILE--
<?php
declare(strict_types=1);

class StrictPredicateArgument implements Stringable {
public function __toString(): string {
throw new Exception('__toString() must not be called');
}
}

foreach (['str_contains', 'str_starts_with', 'str_ends_with'] as $function) {
foreach ([new StrictPredicateArgument, 123] as $invalid) {
foreach ([1, 2] as $position) {
$haystack = $position === 1 ? $invalid : 'abc';
$needle = $position === 2 ? $invalid : 'abc';
$errors = [];
foreach ([false, true] as $dynamic) {
try {
if ($dynamic) {
$function($haystack, $needle);
} else {
match ($function) {
'str_contains' => str_contains($haystack, $needle),
'str_starts_with' => str_starts_with($haystack, $needle),
'str_ends_with' => str_ends_with($haystack, $needle),
};
}
$errors[] = 'unexpected success';
} catch (Throwable $e) {
$errors[] = $e::class . ': ' . $e->getMessage();
}
}
if ($errors[0] !== $errors[1]) {
var_dump($errors);
}
echo $errors[0], "\n";
}
}
}
?>
--EXPECT--
TypeError: str_contains(): Argument #1 ($haystack) must be of type string, StrictPredicateArgument given
TypeError: str_contains(): Argument #2 ($needle) must be of type string, StrictPredicateArgument given
TypeError: str_contains(): Argument #1 ($haystack) must be of type string, int given
TypeError: str_contains(): Argument #2 ($needle) must be of type string, int given
TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, StrictPredicateArgument given
TypeError: str_starts_with(): Argument #2 ($needle) must be of type string, StrictPredicateArgument given
TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, int given
TypeError: str_starts_with(): Argument #2 ($needle) must be of type string, int given
TypeError: str_ends_with(): Argument #1 ($haystack) must be of type string, StrictPredicateArgument given
TypeError: str_ends_with(): Argument #2 ($needle) must be of type string, StrictPredicateArgument given
TypeError: str_ends_with(): Argument #1 ($haystack) must be of type string, int given
TypeError: str_ends_with(): Argument #2 ($needle) must be of type string, int given
Loading