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

Contract does not include reentrancy protection when sending funds

0xVinylDavyl

Summary

The code does not include reentrancy protection when sending funds or making external contract calls. An attacker could potentially exploit this vulnerability to re-enter the contract and manipulate the state.

Vulnerability Details

Consider the selectWinner function:

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);
}

This function performs various actions, including selecting a winner and sending funds to that winner. However, it does not include reentrancy protection, which could allow a malicious external contract to call it repeatedly, potentially draining the contract's balance.

Impact

This function performs various actions, including selecting a winner and sending funds to that winner. However, it does not include reentrancy protection, which could allow a malicious external contract to call it repeatedly, potentially draining the contract's balance.

Tools Used

Manual/VsCode

Recommendations

To mitigate this issue, you should add the nonReentrant modifier from the OpenZeppelin ReentrancyGuard library to functions that perform sensitive state changes. Here's how you can do it:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract PuppyRaffle is ERC721, Ownable, ReentrancyGuard {
// ...
​
function selectWinner() external nonReentrant {
// Existing code
}
​
// ...
}
​

By adding the nonReentrant modifier, you protect the selectWinner function from reentrancy attacks. This ensures that the function can't be called again while it's still processing, preventing potential exploits.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other
0xVinylDavyl Submitter
almost 2 years ago
patrickalphac Lead Judge
almost 2 years ago
Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other
Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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