[GIT-PULLS] [php-src] PR #23436: Fix UAF with pipe operator + namespaced frameless icall
[email protected] (Mrmaxmeier)
| Newsgroups | php.git-pulls |
|---|---|
| Message-ID | <[email protected]> |
Pull Request: https://github.com/php/php-src/pull/23436
Author: Mrmaxmeier
Hi,
we ran into a use-after-free with the `php-fuzz-parser` fuzzing target:
```php
<?php
namespace N;
var_dump(' string literal ' |> trim(...));
```
<details>
<summary>ASAN backtrace for minimal reproducer above</summary>
```
/out/php-fuzz-parser: Running 1 inputs 100 time(s) each.
Running: /testcase
=================================================================
==14==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ba795225e10 at pc 0x55a64fea1b7c bp 0x7fff1f8be110 sp 0x7fff1f8be108
READ of size 4 at 0x7ba795225e10 thread T0
SCARINESS: 45 (4-byte-read-heap-use-after-free)
#0 0x55a64fea1b7b in zend_gc_delref /src/php-src/Zend/zend_types.h:835:2
#1 0x55a64fea1b7b in zval_delref_p /src/php-src/Zend/zend_types.h:1406:9
#2 0x55a64fea1b7b in zval_ptr_dtor_nogc /src/php-src/Zend/zend_variables.h:34:35
#3 0x55a64fea1b7b in destroy_op_array /src/php-src/Zend/zend_opcode.c:614:4
#4 0x55a64ff14556 in fuzzer_do_request_from_buffer /src/php-src/sapi/fuzzer/fuzzer-sapi.c:296:4
#5 0x55a64ff1383a in LLVMFuzzerTestOneInput /src/php-src/sapi/fuzzer/fuzzer-parser.c:33:2
[..]
DEDUP_TOKEN: zend_gc_delref--zval_delref_p--zval_ptr_dtor_nogc
0x7ba795225e10 is located 0 bytes inside of 48-byte region [0x7ba795225e10,0x7ba795225e40)
freed by thread T0 here:
#0 0x55a64eba9b56 in free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:51:3
#1 0x55a64fea0c32 in zval_ptr_dtor_nogc /src/php-src/Zend/zend_variables.h:35:3
#2 0x55a64fea0c32 in destroy_op_array /src/php-src/Zend/zend_opcode.c:614:4
#3 0x55a64ff14556 in fuzzer_do_request_from_buffer /src/php-src/sapi/fuzzer/fuzzer-sapi.c:296:4
#4 0x55a64ff1383a in LLVMFuzzerTestOneInput /src/php-src/sapi/fuzzer/fuzzer-parser.c:33:2
[..]
DEDUP_TOKEN: __interceptor_free--zval_ptr_dtor_nogc--destroy_op_array
previously allocated by thread T0 here:
#0 0x55a64eba9df4 in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:67:3
#1 0x55a64f8e0231 in tracked_malloc /src/php-src/Zend/zend_alloc.c:3016:14
#2 0x55a64fe31b27 in zend_string_alloc /src/php-src/Zend/zend_string.h:190:36
#3 0x55a64fe31b27 in zend_string_init /src/php-src/Zend/zend_string.h:212:21
#4 0x55a64fe46a04 in lex_scan /src/php-src/Zend/zend_language_scanner.l:2604:2
#5 0x55a64f96d860 in zendlex /src/php-src/Zend/zend_compile.c:2064:8
#6 0x55a64fe21231 in zendparse /src/php-src/Zend/zend_language_parser.c:5217:16
#7 0x55a64fe33a1a in zend_compile /src/php-src/Zend/zend_language_scanner.l:644:7
#8 0x55a64fe33734 in compile_file /src/php-src/Zend/zend_language_scanner.l:676:14
#9 0x55a64f03bfd9 in opcache_compile_file /src/php-src/ext/opcache/ZendAccelerator.c
#10 0x55a64f039a4b in persistent_compile_file /src/php-src/ext/opcache/ZendAccelerator.c:2383:23
#11 0x55a64ff144fe in fuzzer_do_request_from_buffer /src/php-src/sapi/fuzzer/fuzzer-sapi.c:289:29
#12 0x55a64ff1383a in LLVMFuzzerTestOneInput /src/php-src/sapi/fuzzer/fuzzer-parser.c:33:2
[..]
DEDUP_TOKEN: __interceptor_malloc--tracked_malloc--zend_string_alloc
SUMMARY: AddressSanitizer: heap-use-after-free /src/php-src/Zend/zend_types.h:835:2 in zend_gc_delref
==14==ABORTING
```
</details>
The issue occurs when namespace and constants are combined with a pipe operator and "frameless" handler functions like `trim()` and `strtolower()`. This reproduces both on `master` and the `PHP-8.5` branch (the pipe operator was added in 8.5).
<details>
<summary>Root cause and fix</summary>
The current logic works like this:
- `zend_compile_pipe()` compiles the left hand side down to a value first and passes it on as a `ZEND_AST_ZNODE` argument.
- Compiling a `ZEND_AST_ZNODE` is a plain struct copy (`*result = *zend_ast_get_znode(ast)`), so every compilation hands out the *same* single reference the node holds. `ZEND_AST_ZVAL` does a `ZVAL_COPY()` instead.
- `SET_NODE()` passes that zval to `zend_add_literal()`, which takes over the reference: `zend_insert_literal()` stores it with `ZVAL_COPY_VALUE()` and, for strings, first runs `zval_make_interned_string()`.
- Inside a namespace, `zend_compile_ns_call()` compiles the argument list *twice*: once for the `INIT_NS_FCALL_BY_NAME` fallback and a second time for the frameless icall behind `ZEND_JMP_FRAMELESS`.
So with a constant on the left hand side one reference is consumed twice. Depending on what `zend_insert_literal()` does with it, that surfaces in two ways:
- Both literals end up owning the same refcount and `destroy_op_array()` releases it twice. That is the trace above; `namespace N; var_dump(['a', 'b', 'c'] |> implode(...));` reaches it via the array case.
- `zval_make_interned_string()` releases the string when interning returns a different one, so the second compilation drops the last reference while the AST still points at it and `zend_ast_destroy()` walks into freed memory.
The fix takes one extra reference per additional compilation, right before the argument list is compiled the second time, so the frameless optimization can be kept for pipes. Only `IS_CONST` `ZEND_AST_ZNODE` arguments are touched, and unpacked or named arguments never reach this path because `zend_compile_ns_call()` already skips the frameless branch for those.
</details>
Notably, while the fix in this PR seems reasonable to me, it is entirely LLM-generated.
Thanks!
---
_Found by the CISPA Fandango team while triaging findings in oss-fuzz harnesses._