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

Possible Reentrancy Attack In selectWinner() Function

yeahchibyke

Summary

Possible reentrancy attack in selectWinner() function

Vulnerability Details

The selectWinner() function in the original smart contract is vulnerable to a reentrancy attack because it makes an external call to transfer the prize pool to the winner's address before it updates the state of the contract. If the winner's address is a contract, it could potentially call back into the selectWinner() function before it's finished, leading to a reentrancy attack.

Tools Used

Foundry, PhindAI, Remix

Recommendations

The state changes (updating the players, raffleStartTime, and previousWinner variables and deleting the players array) should be made before the external call to transfer the prize pool to the winner's address. This ensures that even if the winner's address is a malicious contract that calls back into the selectWinner() function, it won't be able to manipulate the state of the contract.

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();
​
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;
​
// External call made after all state changes
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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

Give us feedback!