Skip to content

ext/standard: Validate objects before queuing unserialize callbacks - #24433

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/aph-224x
Open

iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/aph-224x

Conversation

@iliaal

@iliaal iliaal commented Oct 19, 2026

Copy link
Copy Markdown
Member

The unserializer now checks each object's closing brace before queuing __unserialize() or __wakeup(), so incomplete objects skip their callback and destructor. Related to #21716, which changes when already-queued callbacks are drained.

@ArtUkrainskiy

Copy link
Copy Markdown
Contributor

On the PR head (ef25b3a) ASan reports a heap-use-after-free here, while the base (fd3a97d) and current master are fine. The outer object has no closing }, so it goes through the new early zval_ptr_dtor(&ary) after finish_nested_data() fails:

<?php
class T { public function __unserialize(array $data): void {} }
class D { public function __destruct() { var_dump(unserialize('R:2;')); } }
// the closing } of the outer object is missing
var_dump(@unserialize('O:1:"T":2:{i:0;s:5:"hello";i:1;O:1:"D":0:{}'));

Run with USE_ZEND_ALLOC=0 on an ASan build. It could go in as a test, too.

@iliaal

iliaal commented Oct 22, 2026

Copy link
Copy Markdown
Member Author

Ah, indeed. A failed record's data array now stays in the unserialize context until var_destroy(), like a queued one, so the destructor can no longer reach an element it already released. The same UAF was reachable before this PR when an element after the nested object is malformed; both cases are now in __unserialize_malformed_nested_destructor.phpt.

Check the closing brace before queuing __unserialize() or __wakeup().
Incomplete object records must use the same cleanup as invalid property
data, including destructor suppression. The __unserialize() data array of
a failed record now stays in the unserialize context until var_destroy()
instead of being released at once, so a destructor of a nested object
cannot resolve a back-reference to an element the array already released.
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.

2 participants