From b195e5006ee4a843a00b86e429aeaa6338e719bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 13 Jan 2024 14:41:47 +0100 Subject: [PATCH 1/2] random/standard: Correctly handle broken engines in php_array_pick_keys --- .../03_randomizer/engine_unsafe_biased.phpt | 14 + .../engine_unsafe_empty_string.phpt | 14 + .../03_randomizer/engine_unsafe_nul.phpt | 322 ++++++++++++++++++ ext/standard/array.c | 30 +- 4 files changed, 379 insertions(+), 1 deletion(-) create mode 100644 ext/random/tests/03_randomizer/engine_unsafe_nul.phpt diff --git a/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt b/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt index 09fbd85b54eb0..9e911e826555b 100644 --- a/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt +++ b/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt @@ -43,6 +43,18 @@ try { echo $e->getMessage(), PHP_EOL; } +try { + var_dump(randomizer()->pickArrayKeys(range(1, 1234), 1)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->pickArrayKeys(range(1, 1234), 10)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + try { var_dump(randomizer()->shuffleBytes('foobar')); } catch (Random\BrokenRandomEngineError $e) { @@ -56,3 +68,5 @@ int(%d) string(2) "ff" Failed to generate an acceptable random number in 50 attempts Failed to generate an acceptable random number in 50 attempts +Failed to generate an acceptable random number in 50 attempts +Failed to generate an acceptable random number in 50 attempts diff --git a/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt b/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt index 01bd293bc0508..df9a5f8e267c0 100644 --- a/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt +++ b/ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt @@ -43,6 +43,18 @@ try { echo $e->getMessage(), PHP_EOL; } +try { + var_dump(randomizer()->pickArrayKeys(range(1, 1234), 1)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->pickArrayKeys(range(1, 1234), 10)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + try { var_dump(randomizer()->shuffleBytes('foobar')); } catch (Random\BrokenRandomEngineError $e) { @@ -56,3 +68,5 @@ A random engine must return a non-empty string A random engine must return a non-empty string A random engine must return a non-empty string A random engine must return a non-empty string +A random engine must return a non-empty string +A random engine must return a non-empty string diff --git a/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt b/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt new file mode 100644 index 0000000000000..9320ae1af8f4a --- /dev/null +++ b/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt @@ -0,0 +1,322 @@ +--TEST-- +Random: Randomizer: Nul engines are correctly handled +--FILE-- +getInt(0, 1234)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->nextInt()); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(bin2hex(randomizer()->getBytes(1))); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->shuffleArray(range(1, 123))); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->pickArrayKeys(range(1, 123), 1)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->pickArrayKeys(range(1, 123), 10)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->shuffleBytes('foobar')); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +?> +--EXPECTF-- +int(0) +int(0) +string(2) "00" +array(123) { + [0]=> + int(2) + [1]=> + int(3) + [2]=> + int(4) + [3]=> + int(5) + [4]=> + int(6) + [5]=> + int(7) + [6]=> + int(8) + [7]=> + int(9) + [8]=> + int(10) + [9]=> + int(11) + [10]=> + int(12) + [11]=> + int(13) + [12]=> + int(14) + [13]=> + int(15) + [14]=> + int(16) + [15]=> + int(17) + [16]=> + int(18) + [17]=> + int(19) + [18]=> + int(20) + [19]=> + int(21) + [20]=> + int(22) + [21]=> + int(23) + [22]=> + int(24) + [23]=> + int(25) + [24]=> + int(26) + [25]=> + int(27) + [26]=> + int(28) + [27]=> + int(29) + [28]=> + int(30) + [29]=> + int(31) + [30]=> + int(32) + [31]=> + int(33) + [32]=> + int(34) + [33]=> + int(35) + [34]=> + int(36) + [35]=> + int(37) + [36]=> + int(38) + [37]=> + int(39) + [38]=> + int(40) + [39]=> + int(41) + [40]=> + int(42) + [41]=> + int(43) + [42]=> + int(44) + [43]=> + int(45) + [44]=> + int(46) + [45]=> + int(47) + [46]=> + int(48) + [47]=> + int(49) + [48]=> + int(50) + [49]=> + int(51) + [50]=> + int(52) + [51]=> + int(53) + [52]=> + int(54) + [53]=> + int(55) + [54]=> + int(56) + [55]=> + int(57) + [56]=> + int(58) + [57]=> + int(59) + [58]=> + int(60) + [59]=> + int(61) + [60]=> + int(62) + [61]=> + int(63) + [62]=> + int(64) + [63]=> + int(65) + [64]=> + int(66) + [65]=> + int(67) + [66]=> + int(68) + [67]=> + int(69) + [68]=> + int(70) + [69]=> + int(71) + [70]=> + int(72) + [71]=> + int(73) + [72]=> + int(74) + [73]=> + int(75) + [74]=> + int(76) + [75]=> + int(77) + [76]=> + int(78) + [77]=> + int(79) + [78]=> + int(80) + [79]=> + int(81) + [80]=> + int(82) + [81]=> + int(83) + [82]=> + int(84) + [83]=> + int(85) + [84]=> + int(86) + [85]=> + int(87) + [86]=> + int(88) + [87]=> + int(89) + [88]=> + int(90) + [89]=> + int(91) + [90]=> + int(92) + [91]=> + int(93) + [92]=> + int(94) + [93]=> + int(95) + [94]=> + int(96) + [95]=> + int(97) + [96]=> + int(98) + [97]=> + int(99) + [98]=> + int(100) + [99]=> + int(101) + [100]=> + int(102) + [101]=> + int(103) + [102]=> + int(104) + [103]=> + int(105) + [104]=> + int(106) + [105]=> + int(107) + [106]=> + int(108) + [107]=> + int(109) + [108]=> + int(110) + [109]=> + int(111) + [110]=> + int(112) + [111]=> + int(113) + [112]=> + int(114) + [113]=> + int(115) + [114]=> + int(116) + [115]=> + int(117) + [116]=> + int(118) + [117]=> + int(119) + [118]=> + int(120) + [119]=> + int(121) + [120]=> + int(122) + [121]=> + int(123) + [122]=> + int(1) +} +array(1) { + [0]=> + int(0) +} +Failed to generate an acceptable random number in 50 attempts +string(6) "oobarf" diff --git a/ext/standard/array.c b/ext/standard/array.c index 19b7dc1f8e8ab..361d83b10df41 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -5833,6 +5833,9 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * * specific offset using linear scan. */ i = 0; randval = algo->range(status, 0, num_avail - 1); + if (EG(exception)) { + return false; + } ZEND_HASH_FOREACH_KEY(ht, num_key, string_key) { if (i == randval) { if (string_key) { @@ -5853,6 +5856,9 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * if (HT_IS_PACKED(ht)) { do { randval = algo->range(status, 0, ht->nNumUsed - 1); + if (EG(exception)) { + return false; + } zv = &ht->arPacked[randval]; if (!Z_ISUNDEF_P(zv)) { ZVAL_LONG(retval, randval); @@ -5862,6 +5868,9 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * } else { do { randval = algo->range(status, 0, ht->nNumUsed - 1); + if (EG(exception)) { + return false; + } b = &ht->arData[randval]; if (!Z_ISUNDEF(b->val)) { if (b->key) { @@ -5894,11 +5903,25 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * zend_bitset_clear(bitset, bitset_len); i = num_req; + int failures = 0; while (i) { randval = algo->range(status, 0, num_avail - 1); - if (!zend_bitset_in(bitset, randval)) { + if (EG(exception)) { + goto fail; + } + if (zend_bitset_in(bitset, randval)) { + /* Use PHP_RANDOM_RANGE_ATTEMPTS instead of the hardcoded 50 for 8.3+. */ + if (++failures > 50) { + if (!silent) { + zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", 50); + } + + goto fail; + } + } else { zend_bitset_incl(bitset, randval); i--; + failures = 0; } } @@ -5922,6 +5945,11 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * free_alloca(bitset, use_heap); return true; + + fail: + free_alloca(bitset, use_heap); + + return false; } /* }}} */ From 467893780f05ce188d9cc76bce43881b3a5e084e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 14 Jan 2024 13:01:00 +0100 Subject: [PATCH 2/2] [ci skip] NEWS --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index bd3224b031d81..ae34f72ec668e 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,10 @@ PHP NEWS - Phar: . Fixed bug #71465 (PHAR doesn't know about litespeed). (nielsdos) +- Random: + . Fixed bug GH-13138 (Randomizer::pickArrayKeys() does not detect broken + engines). (timwolla) + 18 Jan 2024, PHP 8.2.15 - Core: