From db80d2ff1eb29fc3a53f9da77706c485deee2073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 00:25:46 +0200 Subject: [PATCH 1/3] Fix shift in rand_range??() The last generated size is in bytes, whereas the shift is in bits. Multiple the generated size by 8 to correctly handle each byte once. --- ext/random/random.c | 8 ++++---- .../tests/03_randomizer/get_int_user.phpt | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 ext/random/tests/03_randomizer/get_int_user.phpt diff --git a/ext/random/random.c b/ext/random/random.c index f8a94e65ee34e..8a41eef10c4d3 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -95,7 +95,7 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat } if (total_size < sizeof(uint32_t)) { r = algo->generate(status); - result = (result << status->last_generated_size) | r; + result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; @@ -133,7 +133,7 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat } while (total_size < sizeof(uint32_t)) { r = algo->generate(status); - result = (result << status->last_generated_size) | r; + result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; @@ -157,7 +157,7 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat } if (total_size < sizeof(uint64_t)) { r = algo->generate(status); - result = (result << status->last_generated_size) | r; + result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; @@ -195,7 +195,7 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat } while (total_size < sizeof(uint64_t)) { r = algo->generate(status); - result = (result << status->last_generated_size) | r; + result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; diff --git a/ext/random/tests/03_randomizer/get_int_user.phpt b/ext/random/tests/03_randomizer/get_int_user.phpt new file mode 100644 index 0000000000000..5368aeb5d6eca --- /dev/null +++ b/ext/random/tests/03_randomizer/get_int_user.phpt @@ -0,0 +1,20 @@ +--TEST-- +Random: Randomizer: User Engine results are correctly expanded for getInt() +--FILE-- +getInt(0, 0xFFFFFF)))); + +?> +--EXPECT-- +string(8) "01010100" From 5cc5da187d015ab959b3ff894e9af5957bd453dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 00:28:45 +0200 Subject: [PATCH 2/3] Correctly handle user engines returning less than 4 bytes in rand_rangeXX() We need to loop until we accumulate sufficient bytes, instead of just checking once. The version in the rejection loop was already correct. --- ext/random/random.c | 4 ++-- ext/random/tests/03_randomizer/get_int_user.phpt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index 8a41eef10c4d3..cd5c0919cdf36 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -93,7 +93,7 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat if (status->last_unsafe) { return 0; } - if (total_size < sizeof(uint32_t)) { + while (total_size < sizeof(uint32_t)) { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; @@ -155,7 +155,7 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat if (status->last_unsafe) { return 0; } - if (total_size < sizeof(uint64_t)) { + while (total_size < sizeof(uint64_t)) { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; diff --git a/ext/random/tests/03_randomizer/get_int_user.phpt b/ext/random/tests/03_randomizer/get_int_user.phpt index 5368aeb5d6eca..fe0e4492171ce 100644 --- a/ext/random/tests/03_randomizer/get_int_user.phpt +++ b/ext/random/tests/03_randomizer/get_int_user.phpt @@ -8,7 +8,7 @@ $randomizer = new \Random\Randomizer ( { public function generate(): string { - return "\x01\x01\x01\x01"; + return "\x01"; } } ); From 68be9c7ef46bac7f99fd859a01c11473630b94d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 00:35:59 +0200 Subject: [PATCH 3/3] Clean up some repetition in rand_rangeXX() --- ext/random/random.c | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index cd5c0919cdf36..1763842fc0dd6 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -88,19 +88,16 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat size_t total_size = 0; uint32_t count = 0; - result = algo->generate(status); - total_size = status->last_generated_size; - if (status->last_unsafe) { - return 0; - } - while (total_size < sizeof(uint32_t)) { + result = 0; + total_size = 0; + do { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; } - } + } while (total_size < sizeof(uint32_t)); /* Special case where no modulus is required */ if (UNEXPECTED(umax == UINT32_MAX)) { @@ -126,19 +123,16 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat return 0; } - result = algo->generate(status); - total_size = status->last_generated_size; - if (status->last_unsafe) { - return 0; - } - while (total_size < sizeof(uint32_t)) { + result = 0; + total_size = 0; + do { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; } - } + } while (total_size < sizeof(uint32_t)); } return result % umax; @@ -150,19 +144,16 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat size_t total_size = 0; uint32_t count = 0; - result = algo->generate(status); - total_size = status->last_generated_size; - if (status->last_unsafe) { - return 0; - } - while (total_size < sizeof(uint64_t)) { + result = 0; + total_size = 0; + do { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; } - } + } while (total_size < sizeof(uint64_t)); /* Special case where no modulus is required */ if (UNEXPECTED(umax == UINT64_MAX)) { @@ -188,19 +179,16 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat return 0; } - result = algo->generate(status); - total_size = status->last_generated_size; - if (status->last_unsafe) { - return 0; - } - while (total_size < sizeof(uint64_t)) { + result = 0; + total_size = 0; + do { r = algo->generate(status); result = (result << (8 * status->last_generated_size)) | r; total_size += status->last_generated_size; if (status->last_unsafe) { return 0; } - } + } while (total_size < sizeof(uint64_t)); } return result % umax;