Skip to content

bug(hooks): CastObjectHook cannot fall through to default engine behaviour — getResult() after a failed proceed() corrupts the calling VM frame #153

Description

@lisachenko

Summary

Two related defects in CastObjectHook make the documented "proceed to the default handler" pattern unusable for cast types the default handler cannot satisfy:

  1. getResult() reads uninitialized memory after a failed proceed(). The engine hands cast_object an uninitialized scratch zval as the retval slot. zend_std_cast_object_tostring() on PHP 8.4 returns FAILURE for IS_LONG/IS_DOUBLE/_IS_NUMBER without writing the slot (the engine caller is the one that emits the "could not be converted" warning and substitutes 1). Calling getResult() at that point runs getNativeValue()/ReferenceEntry machinery over garbage, which observably unsets local variables in the calling frames.

  2. handle() unconditionally returns Core::SUCCESS, so a FAILURE from the original handler can never be propagated to the engine. The engine caller's default behaviour on failure (diagnostic warning + substitute value) is therefore unreachable: whatever the user handler returns — including an accidental null — is silently installed as the cast result.

Reproduction (PHP 8.4.19 NTS, 8.4 branch, ffi.enable=1, opcache.jit=off)

final class Probe implements ObjectCreateInterface, ObjectCastInterface {
    use ObjectCreateTrait;
    public static function __cast(CastObjectHook $hook): mixed {
        $hook->proceed();            // returns -1 (FAILURE) for numeric casts: retval never written
        $res = $hook->getResult();   // reads the uninitialized slot
        error_log('res=' . var_export($res, true));
        return $res;
    }
}
(new ReflectionClass(Probe::class))->installExtensionHandlers();
var_dump((int) new Probe());

Observed output:

Warning: Undefined variable $res in .../Probe.php on line ...
res=NULL
Warning: Undefined variable $result in .../src/ClassExtension/Hook/CastObjectHook.php on line 56
int(1)

The $res local in the user handler ends up undefined after being assigned, and the corruption propagates outward: $result in CastObjectHook::handle() itself (assigned on line 53) is also undefined by the time line 56 fetches it. The final int(1) is a coincidence of the stale retval bytes, not a computed result. Guarding the call (if ($hook->proceed() === Core::SUCCESS) { $hook->getResult(); }) reproduces none of this — getResult() is fine whenever the slot was actually written (e.g. after a successful _IS_BOOL proceed).

Suggested direction

  • handle() should propagate the original handler's status when the user handler signals fall-through (or at minimum expose the status so a userland handler can behave correctly), instead of hard-coding Core::SUCCESS.
  • getResult() should be documented as valid only after a proceed() that returned SUCCESS — or track the last proceed status and throw/return null instead of dereferencing an unwritten slot.
  • The docblock note in handle() ("the retval slot is uninitialized scratch memory") is exactly the reason getResult() must not read it before something has initialized it; the two methods currently contradict each other.

Context

Found while implementing casts in lisachenko/native-php-matrix (lisachenko/native-php-matrix#17); that library works around it by only calling getResult() after proceed() reports SUCCESS and supplying the engine's substitute value (1/1.0) itself otherwise.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions