From 5e6b90e2cc018e032ed83dc23dbf4b1534401bba Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 12 Apr 2026 12:43:19 -0400 Subject: [PATCH 1/3] Fix GH-21730: Mt19937::__debugInfo() leaks state HashTable when the serialize callback fails (#21733) 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 --- ext/random/engine_mt19937.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); } } /* }}} */ From 1a428e528f9c67d67e0b6bff5b036e04f9f9203e Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 12 Apr 2026 13:51:30 -0400 Subject: [PATCH 2/3] Fix GH-21731: Random\Engine\Xoshiro256StarStar::__unserialize() accepts all-zero state (#21732) The constructor rejects a seed that would leave the internal state all zero, because xoshiro256** with zero state produces 0 on every call forever. The unserialize callback didn't check the same invariant. A caller feeding a crafted serialized payload through __unserialize() ended up with a live engine that returned 0 from every operation. Match the constructor: reject the all-zero state from the unserialize callback too. The Mt19937-aliased __unserialize() wrapper turns the false return into the standard "Invalid serialization data" exception. Closes GH-21731 --- NEWS | 4 ++++ ext/random/engine_xoshiro256starstar.c | 4 ++++ .../xoshiro256starstar_unserialize_zero_state.phpt | 14 ++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 ext/random/tests/02_engine/xoshiro256starstar_unserialize_zero_state.phpt diff --git a/NEWS b/NEWS index 07653ef6a37f..24846881de96 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-21731 (Random\Engine\Xoshiro256StarStar::__unserialize() + accepts all-zero state). (iliaal) + - SPL: . Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent free). (Girgias) diff --git a/ext/random/engine_xoshiro256starstar.c b/ext/random/engine_xoshiro256starstar.c index 1a054362f065..12db8198978d 100644 --- a/ext/random/engine_xoshiro256starstar.c +++ b/ext/random/engine_xoshiro256starstar.c @@ -151,6 +151,10 @@ static bool unserialize(void *state, HashTable *data) } } + if (UNEXPECTED(s->state[0] == 0 && s->state[1] == 0 && s->state[2] == 0 && s->state[3] == 0)) { + return false; + } + return true; } diff --git a/ext/random/tests/02_engine/xoshiro256starstar_unserialize_zero_state.phpt b/ext/random/tests/02_engine/xoshiro256starstar_unserialize_zero_state.phpt new file mode 100644 index 000000000000..6ebcd03e8570 --- /dev/null +++ b/ext/random/tests/02_engine/xoshiro256starstar_unserialize_zero_state.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-21731: Xoshiro256StarStar::__unserialize() must reject the all-zero state +--FILE-- +getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +Invalid serialization data for Random\Engine\Xoshiro256StarStar object From f90e532f368745a2a8128100a69175a0a806ea9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 12 Apr 2026 19:53:12 +0200 Subject: [PATCH 3/3] Fix order in NEWS --- NEWS | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 338e14bee080..9da98b15f204 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,10 @@ PHP NEWS - Curl: . Add support for brotli and zstd on Windows. (Shivam Mathur) +- DOM: + . Fixed bug GH-21566 (Dom\XMLDocument::C14N() emits duplicate xmlns + declarations after setAttributeNS()). (David Carlier) + - Iconv: . Fixed bug GH-17399 (iconv memory leak on bailout). (iliaal) @@ -29,10 +33,6 @@ PHP NEWS . Fix memory leak regression in openssl_pbkdf2(). (ndossche) . Fix a bunch of memory leaks and crashes on edge cases. (ndossche) -- DOM: - . Fixed bug GH-21566 (Dom\XMLDocument::C14N() emits duplicate xmlns - declarations after setAttributeNS()). (David Carlier) - - SPL: . Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent free). (Girgias)