Puppy Raffle

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

Unbounded Nested Loop in PuppyRaffle::enterRaffle Causes Denial of Service as Players Grow

Root + Impact

Description:

Normal Behavior:
The PuppyRaffle::enterRaffle function allows new players to enter the raffle. Before adding a new player, it checks whether the caller has already entered by iterating over the players array using a nested loop.

Specific Issue:
The duplicate-check uses a nested loop that iterates over all pairs of players. This makes the Gas cost grow quadratically (O(n²)) with the number of players. The longer the players array becomes, the more checks a new player has to perform. An attacker can intentionally bloat the players array to make the Gas cost exceed the block Gas limit, permanently preventing new players from entering.

// @> Unbounded nested loop causes O(n²) Gas growth.
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:

  • The Gas cost grows quadratically with the number of players.

  • An attacker can deliberately call enterRaffle() from many addresses to bloat the array faster.

  • Once the Gas cost exceeds the block Gas limit, all future enterRaffle() calls revert.

Impact:

  • Legitimate users can no longer enter the raffle.

  • An attacker can guarantee themselves the win by making the array too large for others to join.

  • The raffle becomes permanently unusable (Denial of Service).

Proof of Concept

The test measures the Gas cost of entering the raffle for two batches of 100 players each. The second batch costs more than 3× the first batch, demonstrating that the Gas cost grows quadratically with the number of players. In a real deployment, once the players array is large enough, the Gas cost will exceed the block Gas limit, causing a permanent DoS.

1st 100 players: ~6252048gas

2nd 100 players: ~18068138 gas

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract DoSEnterRaffleTest is Test {
PuppyRaffle public raffle;
​
function setUp() public {
raffle = new PuppyRaffle(
address(this),
1 ether,
10
);
vm.deal(address(this), 1000 ether);
}
​
function testGasGrowsQuadratically() public {
// 1. Measure Gas for the first batch of 100 players.
uint256 gasBefore = gasleft();
for (uint256 i = 0; i < 100; i++) {
address player = address(uint160(i + 1));
vm.deal(player, 1 ether);
vm.prank(player);
raffle.enterRaffle{value: 1 ether}();
}
uint256 gasFirstBatch = gasBefore - gasleft();
console.log("Gas for first 100 players:", gasFirstBatch);
​
// 2. Measure Gas for the second batch of 100 players.
gasBefore = gasleft();
for (uint256 i = 100; i < 200; i++) {
address player = address(uint160(i + 1));
vm.deal(player, 1 ether);
vm.prank(player);
raffle.enterRaffle{value: 1 ether}();
}
uint256 gasSecondBatch = gasBefore - gasleft();
console.log("Gas for second 100 players:", gasSecondBatch);
​
// 3. Assertion: the second batch should cost significantly more.
assertGt(gasSecondBatch, gasFirstBatch * 3);
}
}

Recommended Mitigation

Option 1 replaces the O(n²) nested loop with an O(1) mapping lookup, eliminating the Gas growth entirely. Option 2 removes the duplicate check altogether, which is acceptable because users can already create new wallets to bypass it. Both options eliminate the DoS risk.

//option1
- 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");
- }
- }
+ require(!hasEntered[msg.sender], "PuppyRaffle: Duplicate player");
+ hasEntered[msg.sender] = true;
​
//option2
- 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");
- }
- }
+ // No duplicate check. Users can enter multiple times.
Updates

Lead Judging Commences

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