Repository navigation
Argument corruption when calling XMLReader::open or XMLReader::XML non-statically with observer active #11548
Description
Description
Run this with the tideways_xhprof extension enabled:
<?php
class ObserverTest {
public function __construct() {
$reader = new XMLReader();
$reader->XML( '<root></root>', null, 0 );
print libxml_get_last_error() . "\n";
}
}
new ObserverTest;The $flags argument to XMLReader::XML() becomes garbage. It is a heap address.
Run under gdb to make it more obvious:
gdb --args php8.2 -d extension=tideways_xhprof xmlreader.php
...
(gdb) source .gdbinit
(gdb) break zim_XMLReader_XML
Function "zim_XMLReader_XML" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (zim_XMLReader_XML) pending.
(gdb) run
Starting program: /usr/bin/php8.2 -d extension=tideways_xhprof xmlreader.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Breakpoint 1, zim_XMLReader_XML (execute_data=0x7ffff4e13170, return_value=0x7fffffffaa48) at ./ext/xmlreader/php_xmlreader.c:1013
1013 {
(gdb) zbacktrace
[0x7ffff4e13170] XMLReader->XML("<root></root>", NULL, 140737301786784) [internal function]
[0x7ffff4e130a0] ObserverTest->__construct() /home/tstarling/src/php/php8.2-8.2.7/xmlreader.php:6
[0x7ffff4e13020] (main) /home/tstarling/src/php/php8.2-8.2.7/xmlreader.php:11
Depending on what bits are set in the heap address passed for $flags, libxml_get_last_error() may return an error.
Setting a hardware watchpoint indicates that this assignment on line 236 of zend_observer.c is overwriting the argument, which at this point is already on the VM stack:
*prev_observed_frame(execute_data) = current_observed_frame;Note that XMLReader::XML() and XMLReader::open() are special in that they can be called either statically or non-statically. The bug occurs when calling them non-statically.
Downstream bug: https://phabricator.wikimedia.org/T330464
PHP Version
PHP 8.2.7
Operating System
Ubuntu 22.04
Activity
damianwadley commentedon Jun 28, 2023
Confirmed, but if a bug only reproduces when an extension is installed...
Suggests the bug is in the extension.
And I can't help but notice that the extension hasn't been updated since PHP 8.0...
tstarling commentedon Jun 28, 2023
I reproduced it with the zend_test extension on the current git master:
Girgias commentedon Jun 28, 2023
Assigning @bwoebi as they know something about the observer API AFAIK
bwoebi commentedon Jun 30, 2023
That's really the worst hack :-)
XMLReader::open and XMLReader::XML are copied in minit, then returned directly in xmlreader_get_method.
The functions not being properly registered causes them to be skipped over in zend_observer_post_startup, which adds the temporary for all registered functions.
Fix GH-11548 (Argument corruption when calling XMLReader::open or XML…