It's not clear to the auditor what the purpose of choosing a random tokenId through randomIndex() is instead of a counter.
If lower values are preferential, for something like a mint number, this can be gamed by sending it in a different block or from a different sender.
There also seem to be code paths that check for cases that would only occur when a hash-collision is detected. As the nonce is incremented in each randomIndex this is extremely unlikely and these code paths can be ignored to save gas.
Impact
None / save gas.
Recommended Mitigation Steps
Double-check if the predictable randomness is really needed.
Handle
cmichel
Vulnerability details
Vulnerability Details
It's not clear to the auditor what the purpose of choosing a random
tokenId
throughrandomIndex()
is instead of a counter. If lower values are preferential, for something like a mint number, this can be gamed by sending it in a different block or from a different sender. There also seem to be code paths that check for cases that would only occur when a hash-collision is detected. As thenonce
is incremented in eachrandomIndex
this is extremely unlikely and these code paths can be ignored to save gas.Impact
None / save gas.
Recommended Mitigation Steps
Double-check if the predictable randomness is really needed.