coin selection code uses unseeded rand() call #1057

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

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

but rand() is never seeded.

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

Probably. It is the only place in the source where rand() is used.

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

    //while (rand() % 3 == 0)
    //    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.

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.

@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().

We don't use rand() anymore but our own insecure_rand() for non-security-critical randomness that does get seeded. Closing this.