First Flight #2: Puppy Raffle

Summary

A reentrancy attack is a type of security vulnerability in smart contracts where an attacker is able to repeatedly call a function before its previous invocation is finished. This can lead to unexpected behavior, such as draining funds from a contract. This vulnerability can happen in the PuppyRaffle::refund due to the updating the state variable players after calling the external contract.

Vulnerability Details

In the refund() function, the potential for a reentrancy attack is present in the line payable(msg.sender).sendValue(entranceFee);. Here, the function sends entranceFee to the msg.sender address. The msg.sender could potentially execute code in its fallback() function that calls back into the refund() function before the state of the refund() function has been fully updated. This could allow the attacker to repeatedly refund before the player's address is set to address(0), potentially allowing them to withdraw the contract balance.

Impact

The potential reentrancy attack in the refund() function can allow the malicious user to drain the contract balance. To demonstrate this we can consider the following scenario:

1. Players enter the raffle.

2. Malicious user creates a contract with fallback() or receive() function. This function calls the refund() function of the PuppyRaffle
   contract.

3. The malicious user enters the raffle.

4. The malicious user calls the refund() function from his contract and execute code in the fallback() function that calls back into the refund() function before the state of the refund() function has been fully updated. This allows the malicious user to repeatedly refund before the player's address is set to address(0), allowing to withdraw the contract balance.

Tools Used

VS Code, Foundry

Recommendations

To prevent reentrancy attacks, you should use the Checks-Effects-Interactions pattern. This ensures that the state of the contract is updated before any external calls are made. In the refund() function, you should set players[playerIndex] to address(0) before sending the entranceFee.
Here's how you might update the refund() function to prevent reentrancy attacks:

In the updated function, players[playerIndex] is set to address(0) before sending the entranceFee, which prevents a reentrancy attack.
Also, it is recommendated to use the nonReentered modifier from the OpenZeppelin's library to protect against the reentrancy attack.
https://docs.openzeppelin.com/contracts/4.x/api/security#ReentrancyGuard-nonReentrant--