First Flight #11: Snek-Raffle
First Flight #11
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Incorrect Rarity Calculation in `fulfillRandomWords` Function

naman1729

Summary

  • In fulfillRandomWords function, Calculation of rarity is not correct as it mentions in documentation that 3 type of rarity with possibility to get those are different like COMMON_RARITY(70), RARE_RARITY(25), and LEGEND_RARITY(5). but, Here it has a possibility to get those nfts are equal which is 33.33% for each.

Vulnerability Details

  • below pointed code is used to calculate rarity of nft which is not correct as per documentation.

  • documentation mentions that 3 type of rarity with possibility to get those are different like COMMON_RARITY(70), RARE_RARITY(25), and LEGEND_RARITY(5).

  • Here, it has a possibility to get those nfts are equal which is 33.33% for each.

@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)
​

POC

  • Here, possibility to get a rondom number with a remainder on divisibility by 3 is 0, 1, 2 is 33.33% for each.

  • Chainlink vrf is not biased to any number, so it makes the possibility to get a random number with a remainder on divisibility by 3 is 33.33% for each.

  • So, it is not correct to calculate rarity of nft by the random number with 3 as divisibility parameter.

  • Random Number is not biased 70 for COMMON_RARITY, 25 for RARE_RARITY, and 5 for LEGEND_RARITY.

  • It not make sense to calculate rarity by % 3 we can use % 100 and then, we create if else condition to set the rarity of nft.

Impact

  • It may impact on the rarity of nft which is not correct as per documentation.

  • It is possible to get a nft with highest rarity more than 5% which is not correct as per documentation.

Tools Used

  • manual review

Recommendations

  • It not make sense to calculate rarity by % 3 we can use % 100 and then, we create if else condition to set the rarity of nft.

  • We can use this approach to calculate rarity of nft.

@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[0] % 100
+ if rarity < COMMON_RARITY:
+ rarity = COMMON
+ elif rarity < COMMON_RARITY + RARE_RARITY:
+ rarity = RARE
+ else:
+ rarity = LEGEND
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
Validated
Assigned finding tags:

Rarity is 1/3 instead of what the docs say

Support

FAQs

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