Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Nested Loops in enterRaffle Leading to Potential Denial of Service

Summary

The enterRaffle function in the provided smart contract utilizes nested loops, which when provided with a large set of players can result in excessive gas consumption, potentially leading to a denial of service.

Vulnerability Details

contract PuppyRaffle {
...
function enterRaffle(address[] memory newPlayers) public payable {
...
// 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");
}
}
}

Due to the nested loops, the computational complexity of this function increases quadratically with the number of new players. This can result in the function consuming an excessive amount of gas and becoming unusable.

Impact

Can lead to denial of service when called with a large set of players, making the enterRaffle function unusable.

POC

  1. Fund the contract with adequate ETH.

  2. Note the balance.

  3. Call the testTooManyPlayersEnter function with 257 players.

  4. The estimated gas usage for this transaction is 31997436.

  5. As the number of players increases, gas consumption increases exponentially.

  6. Notice the extremely high gas usage, which can result in the function being inoperable on many networks due to exceeding gas limits.

Code

function testTooManyPlayersEnter() public {
address[] memory players = new address[](257);
for(uint256 i = 0; i < 257; i++){
players[i] = address(i);
}
puppyRaffle.enterRaffle{value: entranceFee * 257}(players);
assertEq(puppyRaffle.players(256), players[256]);
}

Tools Used

・foundry

Recommendations

Optimize the enterRaffle function by removing the nested loops. One approach to ensure no duplicate players are entered is by using a mapping. Replace the players array with a mapping to track whether an address has entered the raffle.

+ mapping(address => bool) public playersEntered;

And then, in the enterRaffle function:

for(uint256 i = 0; i < players.length; i++) {
- for (uint256 j = i + 1; j < players.length; j++) {
- require(players[i] != players[j], "PuppyRaffle: Duplicate player");
- }
+ require(!playersEntered[players[i]], "Player has already entered");
+ playersEntered[players[i]] = true;
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

denial-of-service-in-enter-raffle

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!