Skip to content

Commit 6fb13bb

Browse files
committed
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 (720e006). 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
1 parent afded3d commit 6fb13bb

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ PHP NEWS
3131
- OpenSSL:
3232
. Fix a bunch of memory leaks and crashes on edge cases. (ndossche)
3333

34+
- Random:
35+
. Fixed bug GH-21730 (Mt19937::__debugInfo() leaks state HashTable when
36+
the serialize callback fails). (iliaal)
37+
3438
- SPL:
3539
. Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent
3640
free). (Girgias)

ext/random/engine_mt19937.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,11 +392,11 @@ PHP_METHOD(Random_Engine_Mt19937, __debugInfo)
392392

393393
if (engine->engine.algo->serialize) {
394394
array_init(&t);
395+
zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t);
395396
if (!engine->engine.algo->serialize(engine->engine.state, Z_ARRVAL(t))) {
396397
zend_throw_exception(NULL, "Engine serialize failed", 0);
397398
RETURN_THROWS();
398399
}
399-
zend_hash_str_add(Z_ARR_P(return_value), "__states", strlen("__states"), &t);
400400
}
401401
}
402402
/* }}} */
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
GH-21730: __debugInfo() exposes the engine state under __states
3+
--FILE--
4+
<?php declare(strict_types = 1);
5+
6+
use Random\Engine\Mt19937;
7+
use Random\Engine\PcgOneseq128XslRr64;
8+
use Random\Engine\Xoshiro256StarStar;
9+
10+
$mt = new Mt19937(1234);
11+
$mtInfo = $mt->__debugInfo();
12+
var_dump(array_key_exists('__states', $mtInfo));
13+
var_dump(count($mtInfo['__states']));
14+
15+
$pcg = new PcgOneseq128XslRr64(1234);
16+
$pcgInfo = $pcg->__debugInfo();
17+
var_dump(array_key_exists('__states', $pcgInfo));
18+
var_dump(count($pcgInfo['__states']));
19+
20+
$xo = new Xoshiro256StarStar(1234);
21+
$xoInfo = $xo->__debugInfo();
22+
var_dump(array_key_exists('__states', $xoInfo));
23+
var_dump(count($xoInfo['__states']));
24+
25+
// The serialized form and __debugInfo both round-trip through the engine's
26+
// serialize callback, so they must produce identical state data.
27+
var_dump($mt->__serialize()[1] === $mtInfo['__states']);
28+
var_dump($pcg->__serialize()[1] === $pcgInfo['__states']);
29+
var_dump($xo->__serialize()[1] === $xoInfo['__states']);
30+
31+
?>
32+
--EXPECT--
33+
bool(true)
34+
int(626)
35+
bool(true)
36+
int(2)
37+
bool(true)
38+
int(4)
39+
bool(true)
40+
bool(true)
41+
bool(true)

0 commit comments

Comments
 (0)