First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`PuppyRaffle::selectWinner` can be exploited by an attacker to win the raffle in an unfair predictability attack

0xrex

Summary

The selectWinner function can be exploited by an attacker using the winnerIndex randomness logic to call the function when the predicted winner index will be their index in the array.

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; // vulnerable line of code
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; //@audit check how random block.difficulty is and if we can exploit the randomness to mint the rarest NFT for ourself.
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

When I call the selectWinner function after correctly predicting what the outcome player index will be, I essentially end up gaming the system by only calling the selectWinner function when the index will be my player index which I can get through the getActivePlayerIndex. So the attack path for this is to deploy my exploit contract, predict the randomness to match a result that's my player index, and then call the selectWinner function. The extra block.timestamp and block.difficulty addition to essentially making it more random won't do much to hinder accurate predictability for this attack.

Tools Used

Manual review

Recommendations

One way and probably the best to prevent this is using the Chainlink VRF function to get a random number rather than doing it manually in the PuppyRaffle::selectWinner function.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years 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.

Give us feedback!