Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

Reuse of Random Numbers

Summary

It was observed that the contract utilizes the same random number obtained from Chainlink's VRF service for two different purposes within the contract's logic. This goes against best security practises.

Vulnerability Details

Notice that the internal fulfillRandomWords() function is using the same random number for calculating the winner address and for discovering the rarity of the Snek NFT that will be minted:

@internal
def fulfillRandomWords(request_id: uint256, random_words: uint256[MAX_ARRAY_SIZE]):
@> index_of_winner: uint256 = random_words[0] % len(self.players)
recent_winner: address = self.players[index_of_winner]
self.recent_winner = recent_winner
self.players = []
self.raffle_state = RaffleState.OPEN
self.last_timestamp = block.timestamp
@> rarity: uint256 = random_words[0] % 3
self.tokenIdToRarity[ERC721._total_supply()] = rarity
log WinnerPicked(recent_winner)
ERC721._mint(recent_winner, ERC721._total_supply())
send(recent_winner, self.balance)

Impact

It goes against best security practises regarding random numbers.

Tools Used

Manual analysis.

Recommendations

It is advised to modify the contract's implementation to request two separate random numbers from the external source for each distinct purpose requiring randomness.

So update the NUM_WORDS constant to request two different random numbers:

-NUM_WORDS: constant(uint32) = 1
+NUM_WORDS: constant(uint32) = 2

And make sure the internal fulfillRandomWords() function uses one of them for selecting the winner and the other for discovering the rarity:

@internal
def fulfillRandomWords(request_id: uint256, random_words: uint256[MAX_ARRAY_SIZE]):
index_of_winner: uint256 = random_words[0] % len(self.players)
recent_winner: address = self.players[index_of_winner]
self.recent_winner = recent_winner
self.players = []
self.raffle_state = RaffleState.OPEN
self.last_timestamp = block.timestamp
- rarity: uint256 = random_words[0] % 3
+ rarity: uint256 = random_words[1] % 3
self.tokenIdToRarity[ERC721._total_supply()] = rarity
log WinnerPicked(recent_winner)
ERC721._mint(recent_winner, ERC721._total_supply())
send(recent_winner, self.balance)
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
EloiManuel Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
EloiManuel Submitter
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.