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)