First Flight #2: Puppy Raffle

Summary

The refund() method in PuppyRaffle contract is vulnerable to reentrancy attack. The attacker can leverege it to steal the contract's balance of ETH.

Vulnerability Details

The refund() method allows the raffle participants to withdraw their ETH and exit the lottery. The method is implemented as such:

First of all, it checks if the caller is indeed the player at a given index. Then the method checks if the player is not an address(0). Then it sends the entranceFee to the player. Finally, it sets the player[playerIndex] to address(0), preventing this player from refunding again.

The issue is that the state update (setting the player to address(0) happens after ETH transfer. This violates the CEI pattern and, together with the lack of any additional reentrancy protection, allows the attacker to abuse the method to drain the contract.

The attack scenario is as follows:

1. 10 players have entered the PuppyRaffle so far with entranceFee (let's assume entranceFee = 1 eth)

2. The attacker enters the raffle through a maliciously prepared smart contract

3. The attacker calls refund() method via the malicious contract

4. When the PuppyRaffle contract transfers 1 ETH to the attacker's contract, malicious receive() method gets executed. This method calls the refund() method again. Notice that the state was not updated yet - the PuppyRaffle contract thinks that the attacker has not refunded their funds yet

5. The reentrancy gets repeated 10 times. The attacker has successfully stolen 10 ETH from the PuppyRaffle contract.

Impact

All funds can be stolen from the contract.

Tools Used

Manual review.

Recommendations

Check the order of operations inside the refund() method to ensure compliance with CEI pattern. Add OpenZeppellin's nonReentrant modifier for extra protection.