Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Each NFT rarity have the same probabilities to be distributed

Description

The winner of the Snek Raffle will receive an NFT. According to the documentation and variables at the top of the contract, here are the probabilities:

There are 3 NFTs that can be won in the snek raffle, each with varying rarity.
1. Brown Snek - 70% Chance to get
2. Jungle Snek - 25% Chance to get
3. Cosmic Snek - 5% Chance to get

However, in fulfillRandomWords during the distribution, the rarity is only chosen using a modulo 3 operation, which gives the same chance for each rarity to be assigned.

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)

Risk

Likelyhood: High

  • Every raffle.

Impact: High

  • All NFTs will have the same probability of being assigned each rarity, breaking the concept of rarity where some NFTs should be rarer than others.

Proof of Concept

  • Simulate a large number of raffles and record the minted NFTs.

  • Check the rarity for each existing NFT and verify that approximately one-third of them are assigned each rarity.

Recommended Mitigation

Take into account each probability to define the rarity. Below is a possible solution:

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 = 3 # unexisting rarity to initialize
+ rarity_calc: uint256 = random_words[0] % 100
+ if rarity_calc > COMMON_RARITY - 1:
+ if rarity_calc > COMMON_RARITY + RARE_RARITY - 1:
+ rarity = LEGEND_RARITY
+ else:
+ rarity = RARE_RARITY
+ else:
+ rarity = COMMON_RARITY
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.