[php-src] PR #24553: Fix use-after-free in Randomizer::shuffleArray() when the engine throws

From: 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)

« previous php.git-pulls (#39715) next »