Puppy Raffle

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

Unbounded O(n^2) duplicate check in `enterRaffle()` causes a gas-cost denial of service

Root + Impact

Description

  • Normal behavior: Users call enterRaffle() to join, and the cost of entering should stay roughly constant regardless of how many players are already in the raffle.

  • Specific issue: After pushing the new players, enterRaffle() runs a nested loop over the whole players array to reject duplicates. The work grows quadratically with the number of existing players, so gas cost climbs with every entrant. An attacker can front-load the array with many addresses to make subsequent entries prohibitively expensive or exceed the block gas limit, locking new participants out.

solidity
// 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:

  • Reason 1: Occurs on every entry once the raffle already has a meaningful number of players — the quadratic loop runs unconditionally.

  • Reason 2: Occurs deliberately when an attacker submits a large batch of addresses early to inflate the gas cost for everyone who enters afterward.

Impact:

  • Impact 1: Later entrants pay dramatically more gas than earlier ones, making participation unfair and eventually unaffordable.

  • Impact 2: Entry can be pushed past the block gas limit, freezing new participation — a denial of service on enterRaffle().

Proof of Concept

This PoC measures the gas cost of two equal batches of 100 players. The first 100 enter into an empty array; the second 100 enter when 100 are already present, so the duplicate loop does far more work. The assertion gasSecond > gasFirst confirms the cost scales with existing players — an attacker who enters first makes everyone after them pay more.
Add to test/PuppyRaffleTest.t.sol and run forge test --mt test_dos_gas -vvv.
solidity
function test_dos_gas() public {
vm.txGasPrice(1);
uint256 n = 100;
address[] memory a = new address[](n);
for (uint160 i = 0; i < n; i++) a[i] = address(i + 1);
uint256 g1 = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * n}(a);
uint256 gasFirst = g1 - gasleft();
address[] memory b = new address[](n);
for (uint160 i = 0; i < n; i++) b[i] = address(i + 1 + n);
uint256 g2 = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * n}(b);
uint256 gasSecond = g2 - gasleft();
console.log("Gas, first 100: ", gasFirst);
console.log("Gas, second 100:", gasSecond);
assert(gasSecond > gasFirst);
}
The logs show the second batch costing several times more gas than the first, confirming the quadratic blow-up.

Recommended Mitigation

Remove the on-chain nested loop and enforce uniqueness with a mapping keyed on a raffle id, giving O(1) duplicate detection per player instead of O(n²) over the whole array. Alternatively, drop the duplicate constraint entirely — a duplicate address is not a security issue because each entry pays its own entranceFee. The mapping approach keeps entry gas constant no matter how many players have already joined, eliminating the DoS.
+ mapping(address => uint256) public addressToRaffleId;
+ uint256 public raffleId = 0;
...
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]);
+ addressToRaffleId[newPlayers[i]] = raffleId;
}
- // 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 only among the newly added players
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(addressToRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
+ }
emit RaffleEnter(newPlayers);
}
Updates

Lead Judging Commences

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