Merge #17293: Add assertion to randrange that input is not 0

a35b6824f3 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 a35b6824f3
  laanwj:
    ACK a35b6824f3
  promag:
    ACK a35b6824f3.

Tree-SHA512: 8fc626cde4b04b918100cb7af28753f25ec697bd077ce0e0c640be0357626322aeea233e3c8fd964ba1564b0fda830b7f5188310ebbb119c113513a4b89952dc
This commit is contained in:
Wladimir J. van der Laan 2019-11-02 11:40:40 +01:00
commit 9641366950
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D

View file

@ -166,6 +166,7 @@ public:
/** Generate a random integer in the range [0..range). */
uint64_t randrange(uint64_t range) noexcept
{
assert(range);
--range;
int bits = CountBits(range);
while (true) {