Puppy Raffle

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

Denial of service

Denial of Service via unbounded duplicate check in PuppyRaffle::enterRaffle makes it cost-prohibitive to enter

Description

The enterRaffle function checks for duplicate players using a nested loop that iterates over the entire players array. This means the gas cost of entering the raffle increases quadratically (O(n^2)) as more players join.

As the players array grows, the gas required to pass the duplicate check will eventually exceed the block gas limit, making it impossible for new players to enter. An attacker can deliberately inflate the array with many addresses to front-run and lock out legitimate participants.

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]);
}
// 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");
@> }
@> }
emit RaffleEnter(newPlayers);
}

Risk

Likelihood:

  • The gas cost increase is guaranteed as the number of players grows -- no attacker is even needed, organic growth causes this

  • An attacker can accelerate this by entering many addresses cheaply (the cost is only entranceFee per address, which they can later refund)

Impact:

  • Later entrants pay significantly more gas than early entrants, creating an unfair advantage for early participants

  • Once the array is large enough, the transaction reverts due to exceeding the block gas limit, completely blocking new entries

  • The raffle becomes unusable and funds may be locked

Proof of Concept

function testDosEnterRaffle() public {
vm.txGasPrice(1);
// First 100 players enter
uint256 numPlayers = 100;
address[] memory players = new address[](numPlayers);
for (uint256 i = 0; i < numPlayers; i++) {
players[i] = address(uint160(i + 10));
}
uint256 gasStartFirst = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * numPlayers}(players);
uint256 gasCostFirst = gasStartFirst - gasleft();
// Next 100 players enter
address[] memory playersTwo = new address[](numPlayers);
for (uint256 i = 0; i < numPlayers; i++) {
playersTwo[i] = address(uint160(i + 10 + numPlayers));
}
uint256 gasStartSecond = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * numPlayers}(playersTwo);
uint256 gasCostSecond = gasStartSecond - gasleft();
console.log("Gas cost for first 100 players: ", gasCostFirst);
console.log("Gas cost for second 100 players: ", gasCostSecond);
// The second batch costs significantly more gas
assert(gasCostSecond > gasCostFirst * 2);
}

Recommended Mitigation

Replace the nested loop duplicate check with a mapping for O(1) lookups:

+ 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++) {
+ require(addressToRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
+ addressToRaffleId[newPlayers[i]] = raffleId;
players.push(newPlayers[i]);
}
- // 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");
- }
- }
emit RaffleEnter(newPlayers);
}
// In selectWinner, after `delete players`:
+ raffleId++;
Updates

Lead Judging Commences

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