[SERVER-72373] Ban `rand` Created: 22/Dec/22  Updated: 29/Oct/23  Resolved: 03/Jan/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.3.0-rc0

Type: Bug Priority: Major - P3
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SERVER-72374 Replace `std::rand` usages in query a... Closed
Related
is related to SERVER-72366 Pass random seed to `srand` for unit ... Closed
is related to SERVER-46514 Normalize host selection for mirrored... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
SERVER-72498 add lint rule vs rand and srand Sub-task Closed Billy Donahue  
Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Service Arch 2022-12-26, Service Arch 2023-01-09
Participants:
Linked BF Score: 135

 Description   

The C rand() function should never be used in server code.

It's a very weak random generator.

It's not guaranteed to be reentrant.

It's hard to control (and easy to forget to control) the seed https://jira.mongodb.org/browse/SERVER-72366

All callers should be changed to use PseudoRandom or equivalent mongo/platform/random.h facilities.

mongo/platform/random.h should provide a way to get a parameter-controlled and injectable seed. Perhaps from a keyed registry of them? For testability, components should be able to use their injectable random seed in peace without worrying about other unrelated components messing up the sequence by adding requests for random numbers from the same source (BF-27216). A global singleton pseudorandom generator should therefore not be created.

There is some server code that uses rand() today, incorrectly.
These should be upgraded to PseudoRandom at least.



 Comments   
Comment by Billy Donahue [ 06/Jan/23 ]

Antonio Fuschetto told it would be possible to add a coverity rule for blacklisting std::rand . That may be a way

SERVER-72498 did it as a regex but I feel it's not perfect. There was one place in the code where we returned rand's address instead of calling it. This is risky to do with standard library functions so it was a bug anyway in a sense.

If coverity can help, that's great. It's perhaps not a big enough problem to do too much more work on though.

Comment by Billy Donahue [ 03/Jan/23 ]

Removed all `rand` from codebase but probably needs a linter to keep it out.
A regex lint will be a little tricky because `rand` is a common identifier.

Comment by Githook User [ 03/Jan/23 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}

Message: SERVER-72373 upgrade all `rand` to `PseudoRandom`
Branch: master
https://github.com/mongodb/mongo/commit/4ce757f499c7854c5029d432f5a473802bca1279

Generated at Thu Feb 08 06:21:36 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.