From 97c6da1dece1ff2b1b77708b106f5e46bc0c8c08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 14 Jan 2024 13:01:29 +0100 Subject: [PATCH 1/2] random/standard: Correctly handle broken engines in php_array_pick_keys (#13138) --- NEWS | 4 + .../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 +- 5 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 ext/random/tests/03_randomizer/engine_unsafe_nul.phpt diff --git a/NEWS b/NEWS index bd3224b031d..ae34f72ec66 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: diff --git a/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt b/ext/random/tests/03_randomizer/engine_unsafe_biased.phpt index 09fbd85b54e..9e911e82655 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 01bd293bc05..df9a5f8e267 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 00000000000..9320ae1af8f --- /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 19b7dc1f8e8..361d83b10df 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 00ea756c939e8f0b3d7f2cbd9059c0373481f73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 14 Jan 2024 13:05:44 +0100 Subject: [PATCH 2/2] random/standard: Adjust #13138 for PHP 8.3 --- .../tests/03_randomizer/engine_unsafe_nul.phpt | 14 ++++++++++++++ ext/standard/array.c | 5 ++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt b/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt index 9320ae1af8f..ff53b83b207 100644 --- a/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt +++ b/ext/random/tests/03_randomizer/engine_unsafe_nul.phpt @@ -61,6 +61,18 @@ try { echo $e->getMessage(), PHP_EOL; } +try { + var_dump(randomizer()->getBytesFromString('123', 10)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + var_dump(randomizer()->getBytesFromString(str_repeat('a', 500), 10)); +} catch (Random\BrokenRandomEngineError $e) { + echo $e->getMessage(), PHP_EOL; +} + ?> --EXPECTF-- int(0) @@ -320,3 +332,5 @@ array(1) { } Failed to generate an acceptable random number in 50 attempts string(6) "oobarf" +string(10) "1111111111" +string(10) "aaaaaaaaaa" diff --git a/ext/standard/array.c b/ext/standard/array.c index 7a82b936f7d..388b15a0879 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -6179,10 +6179,9 @@ PHPAPI bool php_array_pick_keys(const php_random_algo *algo, php_random_status * 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 (++failures > PHP_RANDOM_RANGE_ATTEMPTS) { if (!silent) { - zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", 50); + zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", PHP_RANDOM_RANGE_ATTEMPTS); } goto fail;