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

O(n²) duplicate check in enterRaffle() function allows attacker to permanently brick the raffle with a single transaction

phylax

O(n²) duplicate check in enterRaffle() function allows attacker to permanently brick the raffle with a single transaction

Description

  • Participants can enter the raffle through enterRafflefunction which then checks that the player has not entered privously by looping over the playersarray and check for duplicate addresses.

  • When the playersarray becomes large calling the enterRafflefunction will cost a lot of gas and could even revert.

function enterRaffle(address[] memory newPlayers) public payable {
// @audit - MEDIUM - DoS attack vector - A malicious actor could enter the raffle with a large number of players, causing the duplicate check to consume excessive gas and potentially making the contract unusable for others.
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:

  • Any attacker can trigger this by simply calling enterRafflewith a large batch of addresses.


Impact:

  • As the playersarray grows, legitimate users face exponentially increasing gas costs until enterRaffleexceeds the block gas limit entirely, effectivelly shutting down the raffle entirely and preventing any new participants from joining.

Proof of Concept

In this test the flow is as follows:

  1. The first player enters raffle

  2. A bacth of 1000 addresses enters raffle

  3. Last player enters raffle

Checking the difference in gas usage between the first player and the last player

function testDoSAttackEnterRaffle() public {
// First player enters the raffle
address firstPlayer = makeAddr("firstPlayer");
vm.deal(firstPlayer, initialBalance);
address[] memory firstPlayerArray = new address[](1);
firstPlayerArray[0] = firstPlayer;
uint256 firstGasBefore = gasleft();
raffle.enterRaffle{value: entranceFee}(firstPlayerArray);
uint256 firstGasAfter = gasleft();
uint256 firstGasUsed = firstGasBefore - firstGasAfter;
emit log_named_uint("First player gas used for entering raffle", firstGasUsed);
​
// 1000 players enter the raffle
address[] memory players = new address[](1000);
​
for (uint256 i = 0; i < players.length; i++) {
players[i] = makeAddr(string(abi.encodePacked("player", vm.toString(i))));
vm.deal(players[i], initialBalance);
}
uint256 totalEntranceFee = entranceFee * players.length;
raffle.enterRaffle{value: totalEntranceFee}(players);
​
// Last player enters raffle
address lastPlayer = makeAddr("lastPlayer");
vm.deal(lastPlayer, initialBalance);
address[] memory lastPlayerArray = new address[](1);
lastPlayerArray[0] = lastPlayer;
uint256 lastGasBefore = gasleft();
raffle.enterRaffle{value: entranceFee}(lastPlayerArray);
uint256 lastGasAfter = gasleft();
uint256 lastGasUsed = lastGasBefore - lastGasAfter;
emit log_named_uint("Last player gas used for entering raffle", lastGasUsed);
​
// The gas usage should increase dramatically due to O(n²) duplicate checking
// With ~1000 players, the last player should use significantly more gas
assert(lastGasUsed > firstGasUsed);
​
// Show the ratio to demonstrate the DoS attack potential
uint256 gasRatio = lastGasUsed / firstGasUsed;
emit log_named_uint("Gas ratio (last/first)", gasRatio);
​
// With 1000+ players, the gas cost should be at least 1000x higher
assert(gasRatio >= 1000);
}

Here is the output showing that the last player pay over 6000x more gas than the first player:

First player gas used for entering raffle: 61079
Last player gas used for entering raffle: 418608834
Gas ratio (last/first): 6853

Recommended Mitigation

Using a mappingto check for duplicates and catching the newPlayers.lengthwould save a lot of gas and mitigate this vulnerability:

// Add this mapping at the top with the other state variables
+ mapping(address => bool) public isPlayer;
​
​
function enterRaffle(address[] memory newPlayers) public payable {
+ uint256 newPlayersLength = newPlayers.length; // cache length
- require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
+ require(msg.value == entranceFee * newPlayersLength, "PuppyRaffle: Must send enough to enter raffle");
​
- for (uint256 i = 0; i < newPlayers.length; i++) {
+ for (uint256 i = 0; i < newPlayersLength; i++) {
+ require(!isPlayer[newPlayers[i]], "PuppyRaffle: Duplicate player");
+ isPlayer[newPlayers[i]] = true;
players.push(newPlayers[i]);
}
emit RaffleEnter(newPlayers);
}
Updates

Lead Judging Commences

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