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

You can't use things like block.timestamp, block.difficulty, and msg.sender to generate randomness because they can be known in advance and manipulated

Summary

The selectWinner function relies on the concatenation of block.timestamp, block.difficulty, and msg.sender to generate a random number for purposes of selecting a winner. These variables are predictable and manipulable, so there is no true randomness. Also, anyone can call selectWinner which means they are in control of these variables, so they can wait until a block.timestamp and block.difficulty that is favorable to them winning the drawing (together with their address, which they know) and then call selectWinner. This could also be used to break the intended rarity distribution.

Vulnerability Details

function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
uint256 tokenId = totalSupply();
// We use a different RNG calculate from the winnerIndex to determine rarity
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}

Impact

Tools Used

Players can wait until a block.timestamp and block.difficulty where they will win the drawing (ideally one where they will also get a legendary NFT). Then they can call selectWinner. Players can try to predict in advance whether they will win and request a refund of their entry if they won't win and enter the next drawing instead. You don't have a truly random drawing if you use numbers that can be known in advance.

Recommendations

Use Chainlink's VRF contracts to generate a verifiably random number and then use the mod of players.length to pick a winner with true randomness.

Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

weak-randomness

Root cause: bad RNG Impact: manipulate winner

Support

FAQs

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