From 6fb13bb281eafc7c97df67a21e98ddc4f734375e Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 11 Apr 2026 21:30:59 -0400 Subject: [PATCH] Fix GH-21730: Mt19937::__debugInfo() leaks state HashTable when the serialize callback fails Mt19937::__debugInfo() allocates a temporary HashTable with array_init(&t), calls the engine's serialize callback, and then inserts t into the return value. If the callback returns false, the method throws and hits RETURN_THROWS() before inserting t, so the HashTable leaks. PcgOneseq128XslRr64 and Xoshiro256StarStar alias the same method and share the leak. Niels Dossche fixed the same pattern in __serialize() via GH-20383 (720e0069829). That cleanup didn't touch __debugInfo(). Apply the same reordering here: insert t into return_value first, then let the callback populate it. RETURN_THROWS() then unwinds the return value cleanly. The path is latent in stock PHP because the three built-in serialize callbacks (mt19937, pcg, xoshiro) all return true, so no user code reaches the leak today. I'm fixing it for symmetry with GH-20383 and to keep the pattern from regressing if a future engine grows a failing serialize path. Closes GH-21730 --- NEWS | 4 ++ ext/random/engine_mt19937.c | 2 +- .../tests/02_engine/debuginfo_states.phpt | 41 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 ext/random/tests/02_engine/debuginfo_states.phpt diff --git a/NEWS b/NEWS index 07653ef6a37f..8403ce02ff2c 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,10 @@ PHP NEWS - OpenSSL: . Fix a bunch of memory leaks and crashes on edge cases. (ndossche) +- Random: + . Fixed bug GH-21730 (Mt19937::__debugInfo() leaks state HashTable when + the serialize callback fails). (iliaal) + - SPL: . Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent free). (Girgias) diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index 640099909103..b76e89ac1d6e 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -392,11 +392,11 @@ PHP_METHOD(Random_Engine_Mt19937, __debugInfo) if (engine->engine.algo->serialize) { array_init(&t); + zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t); if (!engine->engine.algo->serialize(engine->engine.state, Z_ARRVAL(t))) { zend_throw_exception(NULL, "Engine serialize failed", 0); RETURN_THROWS(); } - zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t); } } /* }}} */ diff --git a/ext/random/tests/02_engine/debuginfo_states.phpt b/ext/random/tests/02_engine/debuginfo_states.phpt new file mode 100644 index 000000000000..dacae8810513 --- /dev/null +++ b/ext/random/tests/02_engine/debuginfo_states.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-21730: __debugInfo() exposes the engine state under __states +--FILE-- +__debugInfo(); +var_dump(array_key_exists('__states', $mtInfo)); +var_dump(count($mtInfo['__states'])); + +$pcg = new PcgOneseq128XslRr64(1234); +$pcgInfo = $pcg->__debugInfo(); +var_dump(array_key_exists('__states', $pcgInfo)); +var_dump(count($pcgInfo['__states'])); + +$xo = new Xoshiro256StarStar(1234); +$xoInfo = $xo->__debugInfo(); +var_dump(array_key_exists('__states', $xoInfo)); +var_dump(count($xoInfo['__states'])); + +// The serialized form and __debugInfo both round-trip through the engine's +// serialize callback, so they must produce identical state data. +var_dump($mt->__serialize()[1] === $mtInfo['__states']); +var_dump($pcg->__serialize()[1] === $pcgInfo['__states']); +var_dump($xo->__serialize()[1] === $xoInfo['__states']); + +?> +--EXPECT-- +bool(true) +int(626) +bool(true) +int(2) +bool(true) +int(4) +bool(true) +bool(true) +bool(true)