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

H-1 DOS via enterRaffle Duplicate Check Loop

thisisfinesec

H-1 DOS via enterRaffle Duplicate Check Loop

Description

The enterRaffle function allows users to register multiple players by passing an array of addresses. After adding the new players, it performs a nested loop over the entire players array to check for duplicates.

The duplicate check runs two nested loops over the full player list, resulting in O(n²) gas complexity relative to the total number of players. As the player count grows, the gas cost of this check increases quadratically. Once the player count is large enough, the gas cost exceeds the block gas limit and enterRaffle becomes permanently uncallable.

// src/PuppyRaffle.sol:86-90
// Check for duplicates
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:

  • A single attacker can spam enterRaffle with many unique addresses via a contract, rapidly inflating the player count at low cost (only entranceFee per entry, refunded later via refund).

Impact:

  • enterRaffle becomes permanently DOSed once the player list exceeds the block gas limit for the nested loop, preventing any new participants from joining the raffle.

  • Existing players who have not yet won are stuck with locked funds until the raffle duration expires and a winner is drawn (if selectWinner can still be called).

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
import "forge-std/Test.sol";
import "../src/PuppyRaffle.sol";
​
contract DOSAttackTest is Test {
PuppyRaffle raffle;
address feeAddress = address(0xFEE);
uint256 entranceFee = 0.1 ether;
uint256 raffleDuration = 3600;
​
function setUp() public {
raffle = new PuppyRaffle(entranceFee, feeAddress, raffleDuration);
}
​
function testDOSViaPlayerSpam() public {
// Attacker calls enterRaffle repeatedly to inflate the player list.
// Each call adds 1 unique address. After enough calls, the nested
// duplicate check will exceed the block gas limit.
//
// The attacker can use a contract to automate this:
for (uint256 i = 0; i < 500; i++) {
address[] memory players = new address[](1);
players[0] = address(uint160(i + 1000));
raffle.enterRaffle{value: entranceFee}(players);
}
​
// At this point, new legitimate users attempting to enter will
// hit out-of-gas on the duplicate check loop.
​
// Demonstrate the gas cost growing:
address[] memory newPlayers = new address[](1);
newPlayers[0] = address(0xBAD);
​
(bool success,) = address(raffle).call(
abi.encodeWithSignature("enterRaffle(address[])", newPlayers)
);
// With enough players, success will be false (out of gas)
assertTrue(success, "enterRaffle should still work at 501 players");
​
// The point at which it breaks depends on block gas limit,
// but the quadratic growth is the core issue.
}
}

Recommended Mitigation

- // Check for duplicates
- 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");
- }
- }
+ // Check for duplicates using a mapping
+ mapping(address => bool) memory seen = mapping(address => bool);
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(newPlayers[i] != address(0), "PuppyRaffle: Invalid address");
+ require(!seen[newPlayers[i]], "PuppyRaffle: Duplicate player in input");
+ require(!playerIsRegistered[newPlayers[i]], "PuppyRaffle: Player already registered");
+ seen[newPlayers[i]] = true;
+ playerIsRegistered[newPlayers[i]] = true;
+ }

Alternatively, replace the players array with an EnumerableSet or maintain a mapping(address => bool) alongside the array to check duplicates in O(1) per new player instead of O(n²) over the full list.

Updates

Lead Judging Commences

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