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

selectWinner() and refund() are vulnerable to front-running: a losing player can monitor the mempool and call refund() before selectWinner() confirms, exiting the raffle and stealing the fee pool

virgilbb

Root + Impact

There is no mechanism preventing a player from calling refund() in the same block as selectWinner(). A player who identifies they are not the winner (by simulating selectWinner off-chain) can front-run the winner selection with a refund() transaction, exit the raffle for free, and simultaneously manipulate totalAmountCollected inside selectWinner() — reducing the prize and the fee the owner receives.

Description

  • selectWinner() computes totalAmountCollected = players.length * entranceFee at execution time, but players entries are set to address(0) when a player refunds — they are not removed. The length stays the same, so totalAmountCollected includes the refunded slot, but the ETH has already left the contract.

  • A front-runner who calls refund() just before selectWinner() confirms:

    1. Gets their entrance fee back

    2. Causes selectWinner() to overestimate totalAmountCollected — it charges the contract for ETH it no longer holds, sending the winner and owner shares that sum to more than the actual balance, or causing a revert

  • Additionally, if a player can predict they will not win (deterministic randomness based on block.timestamp and msg.sender), they can avoid any loss by front-running to exit before the winner is drawn.

// src/PuppyRaffle.sol
function selectWinner() external {
// ...
uint256 totalAmountCollected = players.length * entranceFee; // @> includes refunded(0) slots
uint256 prizePool = (totalAmountCollected * 80) / 100;
// ...
(bool success,) = winner.call{value: prizePool}(""); // may revert if ETH already gone
}
​
function refund(uint256 playerIndex) public {
// No time lock — callable at any time, including same block as selectWinner
address playerAddress = players[playerIndex];
require(playerAddress == msg.sender, "PuppyRaffle: Only the player can refund");
payable(msg.sender).sendValue(entranceFee);
players[playerIndex] = address(0); // @> sets to 0 but length unchanged
}

Risk

Likelihood:

  • Requires MEV infrastructure or manual mempool monitoring, which is accessible to any sophisticated actor. The deterministic randomness (block.timestamp + msg.sender) makes winner prediction trivial.

Impact:

  • A losing player avoids their loss at no cost. The owner's fee is reduced or the entire selectWinner() call reverts, requiring all remaining players to wait for another draw or call refund() themselves — effectively a griefing DoS on the raffle completion.

Proof of Concept

Alice predicts she will not win by simulating selectWinner() with the current block parameters. She front-runs with refund(), recovers her fee, and the subsequent selectWinner() call tries to transfer ETH it no longer holds.

function test_frontRunRefundBeforeSelectWinner() public {
// 4 players enter
address[] memory players = new address[](4);
for (uint i = 0; i < 4; i++) players[i] = address(uint160(i + 1));
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.warp(block.timestamp + duration + 1);
​
// Alice predicts she's not the winner, front-runs with refund
vm.prank(players[0]);
puppyRaffle.refund(0); // Alice exits — ETH leaves contract
​
// selectWinner still counts Alice's slot in totalAmountCollected
// contract balance < computed prizePool → winner transfer fails or shortfalls owner
uint256 contractBalance = address(puppyRaffle).balance;
uint256 computedTotal = 4 * entranceFee; // still uses length=4
assertLt(contractBalance, computedTotal); // contract holds less than it tries to distribute
}

The contract attempts to distribute more ETH than it holds, confirming the front-running attack breaks winner selection.

Recommended Mitigation

Add a time lock that prevents refund() after raffleStartTime + raffleDuration, and compute totalAmountCollected from the actual contract balance rather than players.length * entranceFee:

function refund(uint256 playerIndex) public {
+ require(block.timestamp < raffleStartTime + raffleDuration, "PuppyRaffle: Raffle ended");
// ...
}
​
function selectWinner() external {
- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = address(this).balance;
// ...
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-07] Potential Front-Running Attack in `selectWinner` and `refund` Functions

## Description Malicious actors can watch any `selectWinner` transaction and front-run it with a transaction that calls `refund` to avoid participating in the raffle if he/she is not the winner or even to steal the owner fess utilizing the current calculation of the `totalAmountCollected` variable in the `selectWinner` function. ## Vulnerability Details The PuppyRaffle smart contract is vulnerable to potential front-running attacks in both the `selectWinner` and `refund` functions. Malicious actors can monitor transactions involving the `selectWinner` function and front-run them by submitting a transaction calling the `refund` function just before or after the `selectWinner` transaction. This malicious behavior can be leveraged to exploit the raffle in various ways. Specifically, attackers can: 1. **Attempt to Avoid Participation:** If the attacker is not the intended winner, they can call the `refund` function before the legitimate winner is selected. This refunds the attacker's entrance fee, allowing them to avoid participating in the raffle and effectively nullifying their loss. 2. **Steal Owner Fees:** Exploiting the current calculation of the `totalAmountCollected` variable in the `selectWinner` function, attackers can execute a front-running transaction, manipulating the prize pool to favor themselves. This can result in the attacker claiming more funds than intended, potentially stealing the owner's fees (`totalFees`). ## Impact - **Medium:** The potential front-running attack might lead to undesirable outcomes, including avoiding participation in the raffle and stealing the owner's fees (`totalFees`). These actions can result in significant financial losses and unfair manipulation of the contract. ## Recommendations To mitigate the potential front-running attacks and enhance the security of the PuppyRaffle contract, consider the following recommendations: - Implement Transaction ordering dependence (TOD) to prevent front-running attacks. This can be achieved by applying time locks in which participants can only call the `refund` function after a certain period of time has passed since the `selectWinner` function was called. This would prevent attackers from front-running the `selectWinner` function and calling the `refund` function before the legitimate winner is selected.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!