Skip to content

Fix use-after-free when ArrayObject sort comparator replaces backing store#22310

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/spl-array-sort-uaf
Open

Fix use-after-free when ArrayObject sort comparator replaces backing store#22310
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/spl-array-sort-uaf

Conversation

@iliaal

@iliaal iliaal commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

spl_array_method() caches the backing HashTable pointer across the user-supplied sort comparator. A comparator that re-enters __construct() (or __unserialize()) routes through spl_array_set_array(), which unlike the other ArrayObject mutators had no nApplyCount guard, so it swaps intern->array out from under the cached pointer and the post-sort cleanup releases then dereferences freed memory. Reachable from pure userland; valgrind reports an invalid read and the process segfaults. Add the nApplyCount guard the dimension writers and exchangeArray() already use.

$ao = new ArrayObject([3, 1, 2]);
$o = new ArrayObject([9, 8, 7]);
$ao->uasort(function ($a, $b) use ($ao, $o) {
    static $n = 0;
    if ($n++ === 0) { $ao->__construct($o); }
    return $a <=> $b;
});
var_dump($ao->getArrayCopy()); // segfaults before the patch

…store

spl_array_method() caches the backing HashTable pointer across a
user-supplied comparator (uasort/uksort and the sort handlers). The
comparator can re-enter __construct() or __unserialize(), which route
through spl_array_set_array() and swap intern->array out from under the
cached pointer, leaving the post-sort cleanup to release and dereference
freed memory. Mirror the nApplyCount guard the other mutators already
use so replacing the backing store during a sort throws instead.
@Girgias

Girgias commented Jun 14, 2026

Copy link
Copy Markdown
Member

This feature is deprecated because it is full of bugs. Just ignore it.

Ah, this is not about using an object as a backing storage...

Well SPL still is buggy AF.

@Girgias Girgias self-assigned this Jun 14, 2026

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

If CI is green

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