From b949b88b33ea334eb7ca7c8a8f63273da8d64619 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 1 Jun 2026 07:17:03 -0400 Subject: [PATCH] Pin $this in zend_call_function zend_call_function() ran the callback with fci_cache->object as $this without holding a reference, so a callback that released the last reference to its own receiver (an autoloader unregistering itself, a SQLite3 authorizer calling setAuthorizer(null)) freed $this while its frame was still executing. Take the reference where the receiver is selected rather than after argument setup, so the error handlers reachable from the deprecation notice and the by-reference warning cannot free it first, and release it on every early exit. Fixes GH-22060 Fixes GH-22122 --- Zend/zend_execute_API.c | 19 +++++++++ ext/pdo_sqlite/tests/gh22122.phpt | 40 +++++++++++++++++++ ext/spl/tests/autoloading/gh22060.phpt | 27 +++++++++++++ .../tests/autoloading/gh22060_deprecated.phpt | 36 +++++++++++++++++ ext/sqlite3/tests/gh22122.phpt | 40 +++++++++++++++++++ tests/output/gh20352.phpt | 3 -- 6 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 ext/pdo_sqlite/tests/gh22122.phpt create mode 100644 ext/spl/tests/autoloading/gh22060.phpt create mode 100644 ext/spl/tests/autoloading/gh22060_deprecated.phpt create mode 100644 ext/sqlite3/tests/gh22122.phpt diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index ada9a3ac75b3..ac1683bb368f 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -823,6 +823,7 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ zend_function *func; uint32_t call_info; void *object_or_called_scope; + zend_object *pinned_this = NULL; ZVAL_UNDEF(fci->retval); @@ -866,12 +867,17 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ } else { object_or_called_scope = fci_cache->object; call_info = ZEND_CALL_TOP_FUNCTION | ZEND_CALL_DYNAMIC | ZEND_CALL_HAS_THIS; + pinned_this = fci_cache->object; + GC_ADDREF(pinned_this); } if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_DEPRECATED)) { zend_deprecated_function(func); if (UNEXPECTED(EG(exception))) { + if (pinned_this) { + OBJ_RELEASE(pinned_this); + } return SUCCESS; } } @@ -880,6 +886,9 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) { zend_call_stack_size_error(); zend_release_fcall_info_cache(fci_cache); + if (pinned_this) { + OBJ_RELEASE(pinned_this); + } return SUCCESS; } #endif @@ -917,6 +926,9 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ } zend_vm_stack_free_call_frame(call); zend_release_fcall_info_cache(fci_cache); + if (pinned_this) { + OBJ_RELEASE(pinned_this); + } return SUCCESS; } } @@ -1010,6 +1022,9 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ if (zend_handle_undef_args(call) == FAILURE) { zend_vm_stack_free_args(call); zend_vm_stack_free_call_frame(call); + if (pinned_this) { + OBJ_RELEASE(pinned_this); + } return SUCCESS; } } @@ -1096,6 +1111,10 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_ } EG(fake_scope) = orig_fake_scope; + if (pinned_this) { + OBJ_RELEASE(pinned_this); + } + zend_vm_stack_free_call_frame(call); if (UNEXPECTED(EG(exception))) { diff --git a/ext/pdo_sqlite/tests/gh22122.phpt b/ext/pdo_sqlite/tests/gh22122.phpt new file mode 100644 index 000000000000..ae15d7490ed2 --- /dev/null +++ b/ext/pdo_sqlite/tests/gh22122.phpt @@ -0,0 +1,40 @@ +--TEST-- +GH-22122 (Use-after-free in Pdo\Sqlite authorizer when callback releases the authorizer) +--EXTENSIONS-- +pdo_sqlite +--FILE-- +setAuthorizer(null); + echo "method: ", $this->state, "\n"; + return Pdo\Sqlite::OK; + } +} +$auth = new Auth(); +$db->setAuthorizer([$auth, 'authorize']); +unset($auth); +$db->exec('SELECT 1'); + +$capture = "closure-alive"; +$closure = function (int $action, ...$args) use (&$capture, $db): int { + $db->setAuthorizer(null); + echo "closure: ", $capture, "\n"; + return Pdo\Sqlite::OK; +}; +$db->setAuthorizer($closure); +unset($closure); +$db->exec('SELECT 2'); + +$db->exec('SELECT 3'); +echo "post-disable query ok\n"; +?> +--EXPECT-- +method: alive +closure: closure-alive +post-disable query ok diff --git a/ext/spl/tests/autoloading/gh22060.phpt b/ext/spl/tests/autoloading/gh22060.phpt new file mode 100644 index 000000000000..50dff5d71b11 --- /dev/null +++ b/ext/spl/tests/autoloading/gh22060.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-22060 (Class autoloader $this freed via spl_autoload_unregister during dispatch) +--FILE-- +data, "\n"; + } +} + +$obj = new Loader(); +spl_autoload_register([$obj, 'load']); +unset($obj); + +try { + new NonExistentClass42(); +} catch (\Throwable $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; +} +?> +--EXPECT-- +loader-data +Error: Class "NonExistentClass42" not found diff --git a/ext/spl/tests/autoloading/gh22060_deprecated.phpt b/ext/spl/tests/autoloading/gh22060_deprecated.phpt new file mode 100644 index 000000000000..2132b33c2414 --- /dev/null +++ b/ext/spl/tests/autoloading/gh22060_deprecated.phpt @@ -0,0 +1,36 @@ +--TEST-- +GH-22060 (Autoloader $this freed by the error handler of its own deprecation notice) +--FILE-- +data, "\n"; + } +} + +$obj = new Loader(); +spl_autoload_register([$obj, 'load']); +unset($obj); + +set_error_handler(function (int $no, string $str): bool { + echo $str, "\n"; + foreach (spl_autoload_functions() as $loader) { + spl_autoload_unregister($loader); + } + return true; +}); + +try { + new NonExistentClass42(); +} catch (\Throwable $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; +} +?> +--EXPECT-- +Method Loader::load() is deprecated +loader-data +Error: Class "NonExistentClass42" not found diff --git a/ext/sqlite3/tests/gh22122.phpt b/ext/sqlite3/tests/gh22122.phpt new file mode 100644 index 000000000000..1df1c3bc0e28 --- /dev/null +++ b/ext/sqlite3/tests/gh22122.phpt @@ -0,0 +1,40 @@ +--TEST-- +GH-22122 (Use-after-free in SQLite3 authorizer when callback releases the authorizer) +--EXTENSIONS-- +sqlite3 +--FILE-- +setAuthorizer(null); + echo "method: ", $this->state, "\n"; + return SQLite3::OK; + } +} +$auth = new Auth(); +$db->setAuthorizer([$auth, 'authorize']); +unset($auth); +$db->exec('SELECT 1'); + +$capture = "closure-alive"; +$closure = function (int $action, ...$args) use (&$capture, $db): int { + $db->setAuthorizer(null); + echo "closure: ", $capture, "\n"; + return SQLite3::OK; +}; +$db->setAuthorizer($closure); +unset($closure); +$db->exec('SELECT 2'); + +$db->exec('SELECT 3'); +echo "post-disable query ok\n"; +?> +--EXPECT-- +method: alive +closure: closure-alive +post-disable query ok diff --git a/tests/output/gh20352.phpt b/tests/output/gh20352.phpt index 3074add99d36..16be0b920e80 100644 --- a/tests/output/gh20352.phpt +++ b/tests/output/gh20352.phpt @@ -21,7 +21,4 @@ ob_start(new Test, 1); echo "trigger bug"; ?> --EXPECTF-- -%r(Notice: ob_start\(\): Failed to create buffer in [^\r\n]+ on line \d+\r?\n(\r?\n)?)+%r -Notice: ob_start(): Failed to create buffer in %s on line %d - Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers in %s on line %d