Skip to content

Commit 804c3fc

Browse files
TimWollaTimWolla
authored
Fix byte expansion in rand_rangeXX() (#9056)
* 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. * 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. * Clean up some repetition in rand_rangeXX()
1 parent 998ede7 commit 804c3fc

2 files changed

Lines changed: 40 additions & 32 deletions

File tree

‎ext/random/random.c‎

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,16 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat
8888
size_t total_size = 0;
8989
uint32_t count = 0;
9090

91-
result = algo->generate(status);
92-
total_size = status->last_generated_size;
93-
if (status->last_unsafe) {
94-
return 0;
95-
}
96-
if (total_size < sizeof(uint32_t)) {
91+
result = 0;
92+
total_size = 0;
93+
do {
9794
r = algo->generate(status);
98-
result = (result << status->last_generated_size) | r;
95+
result = (result << (8 * status->last_generated_size)) | r;
9996
total_size += status->last_generated_size;
10097
if (status->last_unsafe) {
10198
return 0;
10299
}
103-
}
100+
} while (total_size < sizeof(uint32_t));
104101

105102
/* Special case where no modulus is required */
106103
if (UNEXPECTED(umax == UINT32_MAX)) {
@@ -126,19 +123,16 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat
126123
return 0;
127124
}
128125

129-
result = algo->generate(status);
130-
total_size = status->last_generated_size;
131-
if (status->last_unsafe) {
132-
return 0;
133-
}
134-
while (total_size < sizeof(uint32_t)) {
126+
result = 0;
127+
total_size = 0;
128+
do {
135129
r = algo->generate(status);
136-
result = (result << status->last_generated_size) | r;
130+
result = (result << (8 * status->last_generated_size)) | r;
137131
total_size += status->last_generated_size;
138132
if (status->last_unsafe) {
139133
return 0;
140134
}
141-
}
135+
} while (total_size < sizeof(uint32_t));
142136
}
143137

144138
return result % umax;
@@ -150,19 +144,16 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat
150144
size_t total_size = 0;
151145
uint32_t count = 0;
152146

153-
result = algo->generate(status);
154-
total_size = status->last_generated_size;
155-
if (status->last_unsafe) {
156-
return 0;
157-
}
158-
if (total_size < sizeof(uint64_t)) {
147+
result = 0;
148+
total_size = 0;
149+
do {
159150
r = algo->generate(status);
160-
result = (result << status->last_generated_size) | r;
151+
result = (result << (8 * status->last_generated_size)) | r;
161152
total_size += status->last_generated_size;
162153
if (status->last_unsafe) {
163154
return 0;
164155
}
165-
}
156+
} while (total_size < sizeof(uint64_t));
166157

167158
/* Special case where no modulus is required */
168159
if (UNEXPECTED(umax == UINT64_MAX)) {
@@ -188,19 +179,16 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat
188179
return 0;
189180
}
190181

191-
result = algo->generate(status);
192-
total_size = status->last_generated_size;
193-
if (status->last_unsafe) {
194-
return 0;
195-
}
196-
while (total_size < sizeof(uint64_t)) {
182+
result = 0;
183+
total_size = 0;
184+
do {
197185
r = algo->generate(status);
198-
result = (result << status->last_generated_size) | r;
186+
result = (result << (8 * status->last_generated_size)) | r;
199187
total_size += status->last_generated_size;
200188
if (status->last_unsafe) {
201189
return 0;
202190
}
203-
}
191+
} while (total_size < sizeof(uint64_t));
204192
}
205193

206194
return result % umax;

‎ext/random/tests/03_randomizer/get_int_user.phpt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Random: Randomizer: User Engine results are correctly expanded for getInt()
3+
--FILE--
4+
<?php
5+
6+
$randomizer = new \Random\Randomizer (
7+
new class () implements \Random\Engine
8+
{
9+
public function generate(): string
10+
{
11+
return "\x01";
12+
}
13+
}
14+
);
15+
16+
var_dump(bin2hex(pack('V', $randomizer->getInt(0, 0xFFFFFF))));
17+
18+
?>
19+
--EXPECT--
20+
string(8) "01010100"

0 commit comments

Comments
 (0)