Puppy Raffle

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

O(n^2) duplicate check in enterRaffle makes entry gas grow quadratically (DoS)

Root + Impact

Description

  • Entering the raffle should cost about the same for every player.

  • The duplicate check compares every pair in the whole players array on every call (O(n^2)). Each new entrant pays for all earlier entries, so gas grows quadratically. An attacker can enter many addresses early to make entry too expensive (or exceed the block gas limit) for everyone after them.

@> 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");
}
}

Attack path:

  1. Right after a round starts, an attacker enters a few hundred fresh addresses in one or more calls.

  2. Every later enterRaffle call must loop over all n^2/2 pairs, so a single new ticket costs millions of gas.

  3. Honest users stop entering (or the call no longer fits in a block); the attacker can refund most of their tickets later, so the griefing is cheap.

Risk

Likelihood:

  • Happens with every growing raffle; deliberate griefing only needs a batch of cheap addresses (refundable afterwards).

Impact:

  • Later players pay much more gas; at large sizes enterRaffle exceeds the block gas limit and nobody can enter. PoC: the first 100 entrants cost ~6.57M gas, the next 100 cost ~19.25M gas.

Proof of Concept

All tests run in a test contract that deploys PuppyRaffle(1e18, address(99), 1 days) and uses the helpers _players(start,n) (distinct addresses) and _enter(p) (pays 1e18 * p.length). Full file: test/AuditPoC.t.sol, forge test --mc AuditPoC passes.

function test_M2_enterRaffleGasGrowsQuadratically() public {
uint256 g0 = gasleft();
_enter(_players(1, 100));
uint256 first = g0 - gasleft();
g0 = gasleft();
_enter(_players(1000, 100));
uint256 second = g0 - gasleft();
emit log_named_uint("gas first 100", first);
emit log_named_uint("gas second 100", second);
assertGt(second, first * 2);
}

Recommended Mitigation

Use a mapping for membership checks (O(1) per new player):

+ mapping(address => uint256) public playerRaffleId;
+ uint256 public raffleId = 1;
...
for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(playerRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
+ playerRaffleId[newPlayers[i]] = raffleId;
players.push(newPlayers[i]);
}
- // nested duplicate loop removed

(increment raffleId in selectWinner).

Updates

Lead Judging Commences

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