Skip to content

Fix GH-20101: SplHeap/SplPriorityQueue serialization exposes INDIRECTs - #20102

Closed
ndossche wants to merge 1 commit into
php:PHP-8.5from
ndossche:fix-20101
Closed

ndossche wants to merge 1 commit into
php:PHP-8.5from
ndossche:fix-20101

Conversation

@ndossche

@ndossche ndossche commented Oct 8, 2025

Copy link
Copy Markdown
Member

Exposing INDIRECTs to userland is not allowed and can lead to all sorts of wrong behaviour. In this case it lead to UAF bugs. Solve it by duplicating the properties table, which de-indirects the elements and also decouples it for future modifications.

@ndossche ndossche linked an issue Oct 8, 2025 that may be closed by this pull request
ndossche added a commit to ndossche/php-src that referenced this pull request Oct 8, 2025
First follow-up to phpGH-20102.
INDIRECTs must never get exposed to userland. The simple solution is to
duplicate the properties array.
@ndossche

ndossche commented Oct 8, 2025 •

Copy link
Copy Markdown
Member Author

I'll have another look after work. It seems that some tests need fixes and/or the implementation-alias wasn't fully right.
EDIT: oh it's probably the true/false difference I didn't see first. I'll fix it after work.

See also ext/spl/tests/SplHeap_serialize_indexed_format.phpt that asserted the buggy behaviour btw.

@Girgias Girgias left a comment

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.

Only into 8.5?

…ECTs

Exposing INDIRECTs to userland is not allowed and can lead to all sorts
of wrong behaviour. In this case it lead to UAF bugs.
Solve it by duplicating the properties table, which de-indirects the
elements and also decouples it for future modifications.
@ndossche

ndossche commented Oct 8, 2025

Copy link
Copy Markdown
Member Author

Only into 8.5?

Yes, because this was only introduced in #19447 which targeted 8.5.

ndossche added a commit that referenced this pull request Oct 8, 2025
First follow-up to GH-20102.
INDIRECTs must never get exposed to userland. The simple solution is to
duplicate the properties array.

Closes GH-20103.
@ndossche ndossche closed this in 0458b3c Oct 8, 2025
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.

SplHeap/SplPriorityQueue serialization exposes INDIRECTs

2 participants