Skip to content

Fix use-after-free of an array callable freed during validation - #23014

Open
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/callable-object-uaf
Open

Fix use-after-free of an array callable freed during validation#23014
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/callable-object-uaf

Conversation

@iliaal

@iliaal iliaal commented Aug 4, 2026

Copy link
Copy Markdown
Member

Resolving a compound [$obj, "Class::method"] callable runs user code before the method is found: the class part may autoload, and the compound form emits an E_DEPRECATED that reaches a user error handler. Either can drop the last reference to the receiver, which zend_is_method_callable() only borrows through fcc->object, so the caller then addrefs and calls into freed memory.

class Victim { public $tag = "alive"; public function target() { echo $this->tag; } }
class Holder extends Victim {}
set_error_handler(function ($n, $s) { if (str_contains($s, 'Callables of the form')) { $GLOBALS['cb'] = null; gc_collect_cycles(); } return true; });
$cb = [new Holder(), 'Victim::target'];
call_user_func($cb); // SIGSEGV

Holding the callable array is not enough: [&$obj, 'Victim::target'] keeps only the reference wrapper alive, and array_map() is affected the same way. The receiver is now held across the compound lookup, and validation fails with "object was destroyed while resolving the callable" when nothing else owns it afterwards, since fcc->object cannot carry ownership back to the caller.

@arnaud-lb

Copy link
Copy Markdown
Member

It seems that fixing #20018 would fix this as well?

@iliaal

iliaal commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Yours does, verified at de280ff (handler runs, callable freed, no crash). #22515 doesn't: it excludes ZEND_INIT_USER_CALL in zend_can_defer_error(), so the deprecation still fires inside zend_is_callable_ex(). Dropping that exclusion from my branch fixes the repro but breaks gh16799 and the gh_21699 trio, so it isn't a one-liner. 8.5 and 8.4 crash on the same repro too.

Comment thread Zend/zend_vm_def.h
ZVAL_COPY(&held_callable, function_name);
function_name = &held_callable;
}
if (zend_is_callable_ex(function_name, NULL, 0, NULL, &fcc, &error)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the refactorings, is it not possible to move this to zend_is_method_callable and do the same sort of zval prevention that we do in increment_string just within the E_DEPRECATED path so that 99.999999% of code doesn't hit this useless performance hit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9d5ab06. The hold can't end the way increment_string's does, because fcc->object is borrowed and the caller only addrefs it after we return. So when the handler leaves the receiver with no other owner, validation fails with "object was destroyed while resolving the callable" instead of calling into it. Moving it there also covers [&$obj, 'C::m'] and array_map(), which the VM-side array hold missed since the array only owned the reference wrapper. Non-compound callables never enter the branch.

Resolving a compound [$obj, "Class::method"] callable can run user code
before the method is found: the class part may autoload, and the compound
form emits an E_DEPRECATED that reaches a user error handler. Either can
drop the last reference to the receiver, which zend_is_method_callable()
only borrows through fcc->object, so the caller then addrefs and calls into
freed memory. Holding the array is not enough, since [&$obj, "C::m"] keeps
only the reference wrapper alive. Hold the receiver across the compound
lookup and fail validation if nothing else owns it afterwards.
@iliaal
iliaal force-pushed the fix/callable-object-uaf branch from 104fb20 to 9d5ab06 Compare September 6, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants