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

[H-01] Unbounded Loops in PuppyRaffle::enterRaffle (Denial of Service Risk)

lyes_boudjabout

Description

The PuppyRaffle::enterRaffle function allows multiple players to enter the raffle at once:

function enterRaffle(address[] memory newPlayers) public payable {
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
​
// Add new players
@> 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);
}

Two unbounded loops exist:

  • Adding newPlayers to the players array.

  • Checking for duplicate entries within the players array.

These loops grow linearly or quadratically with the size of the input arrays plus the existing array's size (players), which can lead to Denial of Service (DoS) if newPlayers is large or if players becomes very long. The transaction may exceed the block gas limit, making it impossible for anyone to enter the raffle.

Risk

  • Severity: High

  • Type: Denial of Service / Gas Limit

  • Impact: Malicious users or large inputs could prevent the raffle from functioning by causing transactions to run out of gas. Duplicate checks on large arrays exacerbate the problem.

Likelihood:

  • Probability of Occurrence: Medium

  • While normal usage may involve small arrays, the risk increases with larger arrays or if an attacker intentionally submits a very large newPlayers array. Quadratic duplicate checks make it feasible for even moderately sized arrays to trigger a DoS.

Impact:

  • Availability: The raffle may become unusable due to gas limits being exceeded.

  • Operational correctness: Users cannot enter the raffle if transactions revert.

  • Maintainability: The quadratic duplicate check is not scalable and may cause hidden issues as the players array grows.

Proof of Concept

A test function demonstrating the issue:

function testDoSEnterRaffle() public {
// Generate three large array of players
address[] memory largePlayers = new address[](100);
address[] memory playersSecond = new address[](100);
address[] memory playersThird = new address[](100);
// Initialize the arrays using dummy addresses
for (uint256 i = 0; i < 100; i++) {
largePlayers[i] = address(i + 1);
playersSecond[i] = address(i + 1 + 100);
playersThird[i] = address(i + 1 + 200);
}
​
// Will be used to call entreRaffle using call function
bytes memory data = abi.encodeWithSelector(
puppyRaffle.enterRaffle.selector,
playersThird
);
​
vm.deal(player, 301 ether);
​
vm.startPrank(player);
puppyRaffle.enterRaffle{value: entranceFee * largePlayers.length}(largePlayers);
puppyRaffle.enterRaffle{value: entranceFee * playersSecond.length}(playersSecond);
// To catch the error in the success variable
(bool success, ) = address(puppyRaffle).call{value: entranceFee * playersThird.length, gas: 2_000_000}(data); // This will exceed gas limits and return false in success due to duplicate check
vm.stopPrank();
​
assertEq(success, false);
}

Explanation

  • Setup large inputs: The test creates three arrays of 100 unique addresses to simulate multiple large raffle entries and grow the players array.

  • Accumulate state: The first two PuppyRaffle::enterRaffle calls succeed, pushing ~200 players into storage, which increases the cost of the nested duplicate-check loop.

  • Force a gas cap: The third call is executed via low-level .call with a fixed gas limit, allowing controlled simulation of an out-of-gas scenario.

  • Trigger OOG via quadratic loop: With ~200 existing players, the duplicate-check loop becomes too expensive and exhausts the provided gas.

  • Assert DoS condition: The call returns success == false, proving the function can be made to fail due to gas exhaustion, enabling a DoS vector.

Recommended Mitigation

  • In PuppyRaffle::enterRaffle, replace the nested duplicate check with a mapping:

- 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");
- }
- }
​
+ mapping(address => bool) private hasEntered; // outside function body
​
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ require(!hasEntered[newPlayers[i]], "PuppyRaffle: Duplicate player");
+ players.push(newPlayers[i]);
+ hasEntered[newPlayers[i]] = true;
+ }

  • Limit the maximum number of players per transaction.

  • Consider batch processing or scalable mechanisms for multiple entries.

  • Document expected array sizes and gas implications for users.

Updates

Lead Judging Commences

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