Skip to content

Fix serialize() object lifetime across nested callbacks#22689

Open
PuH4ck3rX wants to merge 1 commit into
php:PHP-8.4from
PuH4ck3rX:agent/fix-serialize-nested-callback-uaf
Open

Fix serialize() object lifetime across nested callbacks#22689
PuH4ck3rX wants to merge 1 commit into
php:PHP-8.4from
PuH4ck3rX:agent/fix-serialize-nested-callback-uaf

Conversation

@PuH4ck3rX

Copy link
Copy Markdown

php_add_var_hash() skipped uniquely owned objects when the current class had no __serialize() or __sleep() method. However, serializing one of that object's properties can invoke a nested magic callback. The callback may release the current object while its declared-property slots are still being traversed, leaving the serializer with stale zval pointers.

The skipped object was also absent from the serialization identity table. A temporary local reference would prevent destruction but would not preserve reference semantics if a nested callback made the object reachable again. This change removes the unsafe RC1 fast path so objects are retained and registered for the full serialization operation.

The regression test deterministically replaces the released object with a same-layout object. Before the fix, serialize() emits the replacement string under the victim property name; after the fix, it preserves the original integer value.

Checks performed:

  • Built PHP 8.4 with ASan and UBSan, with opcache JIT disabled.
  • The new regression test and the two Cloning an object breaks serialization recursion #12265 identity tests passed.
  • The complete ext/standard/tests/serialize directory passed: 159 passed, 5 skipped, 0 failed.
  • 4 reduced reproducer scripts completed without sanitizer findings.

@PuH4ck3rX PuH4ck3rX marked this pull request as ready for review July 11, 2026 02:54
@PuH4ck3rX PuH4ck3rX requested a review from bukka as a code owner July 11, 2026 02:54
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.

1 participant