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

Reentrancy in refund() - Attacker can steal all funds

Eric

Summary

refund() function does not follow Check-Effects-Interactions pattern (CEI). This can lead to a draining of funds.

Vulnerability Details

Here is the scenario:

  1. An attacker can use a smart contract address to enter the raffle then withdraw their funds.

  2. When they receive the ETH this can then trigger a fallback function that calls 'refund()' again to withdraw more funds

  3. At this point the setting of their entry in the players[] array to address(0)has not happened yet. This means the contract still thinks the player is in the game

  4. Step 2 can be repeated until the balance of the contract is less than 'entranceFee'

Impact

High - this is an easy vector for an attacker to spot and exploit in order to drain ALL or MOST of the contract's ETH balance

Tools Used

Manual Inspection

Recommendations

Follow the CEI pattern and change the reund() function to look like this. We are just swapping two lines, updating the players array and calling sendValue().

function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
​
players[playerIndex] = address(0);
​
payable(msg.sender).sendValue(entranceFee);
​
emit RaffleRefunded(playerAddress);
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

reentrancy-in-refund

reentrancy in refund() function

Support

FAQs

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

Give us feedback!