enterRaffle() re-scans the whole players array with a quadratic duplicate check, so entering becomes prohibitively expensive as the raffle grows
Root + Impact
Description
PuppyRaffle.enterRaffle() in src/PuppyRaffle.sol checks for duplicate players with a nested loop that compares every pair of entries in the whole players array, on every call. The cost of entering therefore grows quadratically with the number of players, and there is no cap on how many players a raffle can have, so entering can eventually become too expensive to use at all.
The vulnerable code is:
The two loops after the push do not just check the new players against the old ones. They re-check every pair in the entire array, including pairs that were already checked when those players entered in earlier transactions. With n players, the number of comparisons is about n * (n - 1) / 2, so doubling the number of players roughly quadruples the work. Each comparison also reads from storage, which is one of the most expensive operations in the EVM. The new players are pushed before the check runs, but a failed check reverts the whole transaction, so that part is not itself a bug; the problem is purely the amount of work the check does.
There is also no limit on players.length and no limit on how many addresses a single call can add. A raffle that is meant to be open-ended therefore has no bound on the cost of the next entry, which is the classic shape of a gas-based denial of service.
Risk
Likelihood: Medium - the cost grows with every entry, so a popular raffle reaches expensive territory through normal use alone, without any attacker. An attacker can also speed this up deliberately by entering many times, which is a permitted action.
Impact: Medium - as the raffle grows, entering becomes more and more expensive, until honest players are priced out or the transaction needs more gas than a block allows, at which point nobody can enter at all and the raffle stalls. The players who already entered cannot be joined by new players, the prize pool stops growing, and the raffle cannot reach the size its owner intended. Funds already paid are not stolen here, but the raffle's availability and fairness are harmed, and gas is wasted on every attempt that reverts.
Proof of Concept
Manual trace - no tests were run; this is a step-by-step reading of the code, with a worked example.
Worked example: count the comparisons the nested loop performs.
With 4 players, the loop compares pairs (0,1), (0,2), (0,3), (1,2), (1,3), (2,3): 6 comparisons.
With 10 players, it performs 45 comparisons, even though only the newest players needed checking.
With 50 players, it performs 1,225 comparisons, and with 100 players it performs 4,950 comparisons, every one of them a storage read and a comparison, on every single entry transaction.
With 500 players, it performs 124,750 comparisons. At that size the gas cost of a single
enterRaffle()call is likely to exceed the block gas limit, so the call reverts no matter how much gas the caller offers, and the raffle can never accept another player.An attacker can reach that state faster by calling
enterRaffle()repeatedly with many addresses, since each call adds players and makes the next call more expensive for everyone else. They do not need any special permission to do this; they only need to pay the entrance fee for each address they add.
Recommended Mitigation
Replace the quadratic scan with a constant-time check. The contract already defines a helper, _isActivePlayer, that loops over the array, so the intended design already has a per-player check; the fix is to use a mapping instead, so each new player is checked in one lookup and past players are never re-checked.
Also clear the mapping entry when a player refunds, and consider a maximum number of players per raffle, so the cost of entering is bounded even if the mapping fix is not applied.
Lead Judging Commences
[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); } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.