coin selection code uses unseeded rand() call #1057

1. 
   dooglus commented at 5:18 pm on April 7, 2012: contributor

   In the code which selects which coins to use, the following appears:

   0if (nPass == 0 ? rand() % 2 : !vfIncluded[i])
   

   but rand() is never seeded.

   Should we use GetRandInt(2) instead to get unpredictable results?

2. 
   laanwj commented at 5:28 pm on April 7, 2012: member

   Probably. It is the only place in the source where rand() is used.
3. 
   dooglus commented at 5:30 pm on April 7, 2012: contributor

   I see it in commented code in main.cpp too:

   0    //while (rand() % 3 == 0)
   1    //    mapNext[pindex->pprev].push_back(pindex);
   

   I wonder if perhaps it’s deliberate, because the wallet rand() code is in a tight loop and we don’t want to exhaust our entropy source.

4. 
   gmaxwell commented at 6:52 pm on April 7, 2012: contributor

   That loop execute a great many times— a proper cryptographically secure RNG is probably going to be noticeably slow here. If it is, then would anyone object to using a fast PRNG seeded by RandBytes at the start of the function?

   In general I don’t think there is a big privacy concern here— even knowing the random sequence completely shouldn’t matter unless you also know the inputs under consideration.

5. laanwj closed this on Apr 9, 2012

   ---

6. laanwj reopened this on Apr 9, 2012

   ---

7. 
   laanwj commented at 7:32 am on April 9, 2012: member

   @gmaxwell also, rand() is not thread-safe according to its man page, using rand_r(*seed) is recommended in that case is as it has explicit state.

   If there is no privacy concern here, a simple solution would be using rand_r with the state (just one int) generated at the beginning of the function with GetRandInt().

8. 
   laanwj commented at 11:13 am on December 22, 2013: member

   We don’t use rand() anymore but our own insecure_rand() for non-security-critical randomness that does get seeded. Closing this.
9. laanwj closed this on Dec 22, 2013

   ---

10. sanch0panza referenced this in commit 4219bb28ac on May 17, 2018
11. lateminer referenced this in commit d4e6525410 on Dec 25, 2019
12. MarcoFalke locked this on Sep 8, 2021

Contributors
dooglus laanwj gmaxwell

Labels
Bug