Skip to content

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Oct 8, 2025

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.

@nielsdos nielsdos linked an issue Oct 8, 2025 that may be closed by this pull request
nielsdos added a commit to nielsdos/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.
@nielsdos
Copy link
Member Author

nielsdos commented Oct 8, 2025

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.

Copy link
Member

@Girgias Girgias left a comment

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.
@nielsdos
Copy link
Member Author

nielsdos commented Oct 8, 2025

Only into 8.5?

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

nielsdos 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.
@nielsdos nielsdos 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