[php-src] PR #24553: Fix use-after-free in Randomizer::shuffleArray() when the engine throws
| From: | Marti2203 | Date: | Thu, 22 Oct 2026 15:55:26 +0000 |
| Subject: | [php-src] PR #24553: Fix use-after-free in Randomizer::shuffleArray() when the engine throws | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-39715@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/24553
Author: Marti2203
php_array_data_shuffle() first compacts the holes of a packed array by moving the
remaining values down with ZVAL_COPY_VALUE(), and only sets nNumUsed after
the shuffle loop:
```c
if (hash->nNumUsed != hash->nNumOfElements) {
for (j = 0, idx = 0; idx < hash->nNumUsed; idx++) {
...
ZVAL_COPY_VALUE(&hash->arPacked[j], zv);
...
}
}
while (--n_left) {
rnd_idx = algo->range(state, 0, n_left);
if (EG(exception)) {
return false; /* nNumUsed still counts the old tail */
}
...
}
...
hash->nNumUsed = n_elems;
```
If the engine throws during the loop, the function returns with the old nNumUsed. The
tail slots still hold copies of values that were moved down, so those values are counted twice.
Randomizer::shuffleArray() then returns with RETURN_THROWS() and its copy
of the array is destroyed, which releases the moved values twice. The caller's original array
is left pointing at a freed value.
This needs a user-supplied Random\Engine that throws (or whose generate()
returns an empty string, which also throws), and an array with holes. shuffle() uses
the default engine, which does not throw, and arrays without holes are not affected.
### Reproducer
```php
<?php
final class ThrowingEngine implements Random\Engine {
public function generate(): string { throw new Exception("boom"); }
}
$a = [new stdClass, new stdClass, new stdClass];
unset($a[1]);
$r = new Random\Randomizer(new ThrowingEngine);
try {
$r->shuffleArray($a);
} catch (Exception $e) {
echo "caught: ", $e->getMessage(), "\n";
}
var_dump($a);
echo "done\n";
```
- Before: release build segfaults in var_dump($a) while printing $a[2]. An
ASan build with USE_ZEND_ALLOC=0 reports heap-use-after-free ... READ of size
8 in php_var_dump(); the object was freed by
zend_objects_store_del() from zend_array_destroy() of the copy.
- After: prints both objects and done, no ASan report.
### Fix
Set nNumUsed to the number of elements right after the compaction, in both the regular
and the iterator branch, so the table is consistent before the engine is called. The success path is
unchanged. It still sets nNumUsed, nInternalPointer and
nNextFreeElement at the end.
### Test
ext/random/tests/03_randomizer/methods/shuffleArray_engine_throws_with_holes.phpt
shuffles an array with a hole, using an engine that throws on the first call and one that throws on
the second call. It checks that the original array is intact afterwards.
- Without the fix: the test crashes (Termsig=11) in the release build, and ASan reports
a heap-use-after-free.
- With the fix: passes in the release build and with ASan (USE_ZEND_ALLOC=0).
- ext/random/tests and ext/standard/tests/array: 909 passed, 14 skipped
(release build). Under ASan, the same set passes except 4 stack-limit tests
(count_recursive_stack_limit.phpt, gh23111.phpt,
gh23113.phpt, gh23115.phpt). They fail the same way without this change.
This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually.
🤖 Generated with [Claude Code](https://claude.com/claude-code)