Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Severity: medium
Valid

[H-1] Unbounded Player Array Enables Denial of Service on enterRaffle()

Root + Impact

  • Root: Unbounded Nested Loop Over Global State

  • Impact: PuppyRaffle::enterRaffle() Becomes Prohibitively Expensive

Description

  • The contract checks for duplicate players using a nested loop over the entire players array.

Both loops iterate over players, which grows unboundedly with each raffle entry.

  • The duplicate check runs after new players are pushed, meaning it checks all existing players against all new players.

  • Because players is a shared global array, any user who enters the raffle increases the gas cost for all future entrants.

  • This creates a flaw where an attacker can fill the players array with hundreds of addresses, making the gas cost of enterRaffle() prohibitively high for all subsequent callers.

for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}

Risk

Likelihood: High

  • The attack requires no special permissions and can be executed by any user

The attacker can recover their entrance fees via refund(), making the net cost nearly zero (only gas)

  • The array never shrinks until selectWinner() is called, so the attack persists for the entire raffle duration

Impact:

  • Any user can make enterRaffle() too expensive for legitimate users

The raffle's core entry function becomes effectively unusable

  • Disrupts the core protocol feature of player participation

  • Attack is cheap to sustain since entrance fees are fully refundable

Proof of Concept

The following test demonstrates how an attacker bloats the array, causing gas costs to increase ~400x for the same single-player entry.

Step-by-step:

  1. Baseline Measurement

    • A single player enters the raffle with a fresh array

    • Gas cost is recorded (~55,735 gas)

  2. Attacker Action

    • Attacker submits 100 addresses via enterRaffle()

    • Attacker can call refund() on each to recover entrance fees

  3. Victim Attempt

    • The same single-player entry is attempted again

    • Gas cost is recorded (~22,535,081 gas)

  4. Result

    • Gas cost increased by ~22,479,346 gas (~400x)

    • This exceeds block gas limits at scale, making entry impossible

function test_dosEnterRaffle() public {
// Phase 1: baseline gas measurement
address[] memory playersPhase1 = new address[](1);
playersPhase1[0] = address(1);
uint256 gasStartPhase1 = gasleft();
puppyRaffle.enterRaffle{value: entranceFee}(playersPhase1);
uint256 gasUsedPhase1 = gasStartPhase1 - gasleft();
// Phase 2: attacker bloats the array
address[] memory attackers = new address[](100);
for (uint256 i = 0; i < 100; i++) {
attackers[i] = address(uint160(i + 2));
}
puppyRaffle.enterRaffle{value: entranceFee * 100}(attackers);
// Phase 3: same single player entry, now with bloated array
address[] memory playersPhase3 = new address[](1);
playersPhase3[0] = address(200);
uint256 gasStartPhase3 = gasleft();
puppyRaffle.enterRaffle{value: entranceFee}(playersPhase3);
uint256 gasUsedPhase3 = gasStartPhase3 - gasleft();
assert(gasUsedPhase3 > gasUsedPhase1);
}

Recommended Mitigation

The current implementation uses a nested loop to check for duplicates, meaning every new entrant must be compared against every existing player. This causes gas costs to grow exponentially as the player array grows — 100 players means 100 checks, 1000 players means 1000 checks, and so on.

The fix is to replace the nested loop with a mapping, which works like a dictionary. Instead of searching through every player, you simply ask "is this address already registered?" and get an instant answer regardless of how many players are in the raffle. The cost stays the same whether there are 10 players or 10,000.

- for (uint256 i = 0; i < players.length - 1; i++) {
- for (uint256 j = i + 1; j < players.length; j++) {
- require(players[i] != players[j], "PuppyRaffle: Duplicate player");
- }
- }
+ mapping(address => bool) public playerExists;
+
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(!playerExists[newPlayers[i]], "PuppyRaffle: Duplicate player");
+ playerExists[newPlayers[i]] = true;
+ }
Updates

Lead Judging Commences

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

[M-01] `PuppyRaffle: enterRaffle` Use of gas extensive duplicate check leads to Denial of Service, making subsequent participants to spend much more gas than prev ones to enter

## Description `enterRaffle` function uses gas inefficient duplicate check that causes leads to Denial of Service, making subsequent participants to spend much more gas than previous users to enter. ## Vulnerability Details In the `enterRaffle` function, to check duplicates, it loops through the `players` array. As the `player` array grows, it will make more checks, which leads the later user to pay more gas than the earlier one. More users in the Raffle, more checks a user have to make leads to pay more gas. ## Impact As the arrays grows significantly over time, it will make the function unusable due to block gas limit. This is not a fair approach and lead to bad user experience. ## POC In existing test suit, add this test to see the difference b/w gas for users. once added run `forge test --match-test testEnterRaffleIsGasInefficient -vvvvv` in terminal. you will be able to see logs in terminal. ```solidity function testEnterRaffleIsGasInefficient() public { vm.startPrank(owner); vm.txGasPrice(1); /// First we enter 100 participants uint256 firstBatch = 100; address[] memory firstBatchPlayers = new address[](firstBatch); for(uint256 i = 0; i < firstBatchPlayers; i++) { firstBatch[i] = address(i); } uint256 gasStart = gasleft(); puppyRaffle.enterRaffle{value: entranceFee * firstBatch}(firstBatchPlayers); uint256 gasEnd = gasleft(); uint256 gasUsedForFirstBatch = (gasStart - gasEnd) * txPrice; console.log("Gas cost of the first 100 partipants is:", gasUsedForFirstBatch); /// Now we enter 100 more participants uint256 secondBatch = 200; address[] memory secondBatchPlayers = new address[](secondBatch); for(uint256 i = 100; i < secondBatchPlayers; i++) { secondBatch[i] = address(i); } gasStart = gasleft(); puppyRaffle.enterRaffle{value: entranceFee * secondBatch}(secondBatchPlayers); gasEnd = gasleft(); uint256 gasUsedForSecondBatch = (gasStart - gasEnd) * txPrice; console.log("Gas cost of the next 100 participant is:", gasUsedForSecondBatch); vm.stopPrank(owner); } ``` ## Recommendations Here are some of recommendations, any one of that can be used to mitigate this risk. 1. User a mapping to check duplicates. For this approach you to declare a variable `uint256 raffleID`, that way each raffle will have unique id. Add a mapping from player address to raffle id to keep of users for particular round. ```diff + uint256 public raffleID; + mapping (address => uint256) public usersToRaffleId; . . function enterRaffle(address[] memory newPlayers) public payable { require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle"); for (uint256 i = 0; i < newPlayers.length; i++) { players.push(newPlayers[i]); + usersToRaffleId[newPlayers[i]] = true; } // Check for duplicates + for (uint256 i = 0; i < newPlayers.length; i++){ + require(usersToRaffleId[i] != raffleID, "PuppyRaffle: Already a participant"); - for (uint256 i = 0; i < players.length - 1; i++) { - for (uint256 j = i + 1; j < players.length; j++) { - require(players[i] != players[j], "PuppyRaffle: Duplicate player"); - } } emit RaffleEnter(newPlayers); } . . . function selectWinner() external { //Existing code + raffleID = raffleID + 1; } ``` 2. Allow duplicates participants, As technically you can't stop people participants more than once. As players can use new address to enter. ```solidity function enterRaffle(address[] memory newPlayers) public payable { require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle"); for (uint256 i = 0; i < newPlayers.length; i++) { players.push(newPlayers[i]); } emit RaffleEnter(newPlayers); } ```

Support

FAQs

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

Give us feedback!