Skip to content

Commit 9641366

Browse files
committed
Merge #17293: Add assertion to randrange that input is not 0
a35b682 Add assertion to randrange that input is not 0 (Jeremy Rubin) Pull request description: From the comment in randrange, their is an implicit argument that randrange cannot accept an argument of 0. If the argument is 0, then we have to return {}, which is not possible in a uint64_t. The current code takes a very interesting approach, which is to return [0..std::numeric_limits<uint64_t>]. This can cause all sorts of fun problems, like allocating a lot of memory, accessing random memory (maybe with your private keys), and crashing the computer entirely. This gives us three choices of how to make it "safe": 1) return Optional<uint64_t> 2) Change the return type to [0..range] 3) Return 0 if 0 4) Assert(range) So which solution is best? 1) seems a bit overkill, as it makes any code using randrange worse. 2) Changing the return type as in 2 could be acceptable, but it imposes the potential overflow checking on the caller (which is what we want). 3) An interesting option -- effective makes the return type in {0} U [0..range]. But this is a bad choice, because it leads to code like `vec[randrange(vec.size())]`, which is incorrect for an empty vector. Null set should mean null set. 4) Assert(range) stands out as the best mitigation for now, with perhaps a future change to solution 2. It prevents the error from propagating at the earliest possible time, so the program crashes cleanly rather than by freezing the computer or accessing random memory. ACKs for top commit: instagibbs: Seems reasonable for now, ACK bitcoin/bitcoin@a35b682 laanwj: ACK a35b682 promag: ACK a35b682. Tree-SHA512: 8fc626cde4b04b918100cb7af28753f25ec697bd077ce0e0c640be0357626322aeea233e3c8fd964ba1564b0fda830b7f5188310ebbb119c113513a4b89952dc
2 parents a5224be + a35b682 commit 9641366

File tree

1 file changed

+1
-0
lines changed

1 file changed

+1
-0
lines changed

src/random.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ class FastRandomContext {
166166
/** Generate a random integer in the range [0..range). */
167167
uint64_t randrange(uint64_t range) noexcept
168168
{
169+
assert(range);
169170
--range;
170171
int bits = CountBits(range);
171172
while (true) {

0 commit comments

Comments
 (0)