Skip to content

Commit 8712f4b

Browse files
nielsdosiluuu1994
andcommitted
Fix OSS-Fuzz #427814452
Pipe compilation uses a temporary znode with QM_ASSIGN to remove references. Assert compilation wants to look at the operand AST and convert it to a string. However the original AST is lost due to the temporary znode. To solve this we either have to handle this specially in pipe compilation [1], or store the AST anyway somehow. Special casing this either way is not worth the complexity in my opinion, especially as it looks like a dynamic call anyway due to the FCC syntax. [1] Prototype (incomplete) at https://gist.github.com/nielsdos/50dc71718639c3af05db84a4dea6eb71 shows this is not worthwhile in my opinion. Closes GH-18965. Co-authored-by: Ilija Tovilo <[email protected]>
1 parent f11ea2a commit 8712f4b

File tree

4 files changed

+57
-1
lines changed

4 files changed

+57
-1
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.5.0alpha2
44

5+
- Core:
6+
. Fix OSS-Fuzz #427814452 (pipe compilation fails with assert). (nielsdos)
7+
58
- DOM:
69
. Make cloning DOM node lists, maps, and collections fail. (nielsdos)
710

Zend/tests/pipe_operator/gh18965.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
GH-18965: ASSERT_CHECK avoids pipe lhs free
3+
--INI--
4+
zend.assertions=0
5+
--FILE--
6+
<?php
7+
namespace Foo;
8+
range(0, 10) |> assert(...);
9+
echo "No leak\n";
10+
?>
11+
--EXPECT--
12+
No leak
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
OSS-Fuzz #427814452
3+
--FILE--
4+
<?php
5+
6+
try {
7+
false |> assert(...);
8+
} catch (\AssertionError $e) {
9+
echo $e::class, ": '", $e->getMessage(), "'\n";
10+
}
11+
try {
12+
0 |> "assert"(...);
13+
} catch (\AssertionError $e) {
14+
echo $e::class, ": '", $e->getMessage(), "'\n";
15+
}
16+
try {
17+
false |> ("a"."ssert")(...);
18+
} catch (\AssertionError $e) {
19+
echo $e::class, ": '", $e->getMessage(), "'\n";
20+
}
21+
22+
?>
23+
--EXPECT--
24+
AssertionError: ''
25+
AssertionError: ''
26+
AssertionError: ''

Zend/zend_compile.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6426,6 +6426,20 @@ static bool can_match_use_jumptable(zend_ast_list *arms) {
64266426
return 1;
64276427
}
64286428

6429+
static bool zend_is_pipe_optimizable_callable_name(zend_ast *ast)
6430+
{
6431+
if (ast->kind == ZEND_AST_ZVAL && Z_TYPE_P(zend_ast_get_zval(ast)) == IS_STRING) {
6432+
/* Assert compilation adds a message operand, but this is incompatible with the
6433+
* pipe optimization that uses a temporary znode for the reference elimination.
6434+
* Therefore, disable the optimization for assert.
6435+
* Note that "assert" as a name is always treated as fully qualified. */
6436+
zend_string *str = zend_ast_get_str(ast);
6437+
return !zend_string_equals_literal_ci(str, "assert");
6438+
}
6439+
6440+
return true;
6441+
}
6442+
64296443
static void zend_compile_pipe(znode *result, zend_ast *ast)
64306444
{
64316445
zend_ast *operand_ast = ast->child[0];
@@ -6453,7 +6467,8 @@ static void zend_compile_pipe(znode *result, zend_ast *ast)
64536467

64546468
/* Turn $foo |> bar(...) into bar($foo). */
64556469
if (callable_ast->kind == ZEND_AST_CALL
6456-
&& callable_ast->child[1]->kind == ZEND_AST_CALLABLE_CONVERT) {
6470+
&& callable_ast->child[1]->kind == ZEND_AST_CALLABLE_CONVERT
6471+
&& zend_is_pipe_optimizable_callable_name(callable_ast->child[0])) {
64576472
fcall_ast = zend_ast_create(ZEND_AST_CALL,
64586473
callable_ast->child[0], arg_list_ast);
64596474
/* Turn $foo |> bar::baz(...) into bar::baz($foo). */

0 commit comments

Comments
 (0)