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

a35b6824f3a0bdb68c5aef599c0f17562689970e 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 https://github.com/bitcoin/bitcoin/pull/17293/commits/a35b6824f3a0bdb68c5aef599c0f17562689970e
  laanwj:
    ACK a35b6824f3a0bdb68c5aef599c0f17562689970e
  promag:
    ACK a35b6824f3a0bdb68c5aef599c0f17562689970e.

Tree-SHA512: 8fc626cde4b04b918100cb7af28753f25ec697bd077ce0e0c640be0357626322aeea233e3c8fd964ba1564b0fda830b7f5188310ebbb119c113513a4b89952dc

0 files changed, 0 insertions, 0 deletions