First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Inefficient Duplicate Check in enterRaffle Function Leads to High Gas Costs and Potential Unusability

charalab0ts

Summary

Vulnerability Details

The PuppyRaffle : : enterRaffle() function in the PuppyRaffle contract uses a nested loop to check for duplicate entries. This approach can lead to high gas costs when the number of players is large.

​
// 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");
}

PoC

Code
function testHighGasCostWithManyPlayers() public {
// Create an array with a large number of players
address[] memory players = new address[](4000);
for (uint i = 0; i < 4000; i++) {
players[i] = address(i);
}
// Measure the gas cost of entering the raffle with this many players
uint256 gasBefore = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * 4000}(players);
uint256 gasAfter = gasleft();
console.log("Gas used to enter raffle with 4000 players: ", gasBefore - gasAfter);

Impact

The inefficiency in the enterRaffle() function could potentially make the contract unusable when dealing with a large number of players due to the high gas costs

Tools Used

Manual

Recommendations

It's recommended to address this issue by implementing a more efficient method for checking duplicate entries, such as using a mapping.

  1. Declare a mapping at the top of your contract like so: mapping(address => bool) public isPlayer;

  2. In the enterRaffle function, before pushing a new player into the players array, check if the player is already in the isPlayer mapping. If they are, revert the transaction. If they're not, add them to the isPlayer mapping and then push them into the players array.

Here's what the updated enterRaffle function might look like:

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(!isPlayer[newPlayers[i]], "PuppyRaffle: Duplicate player");
isPlayer[newPlayers[i]] = true;
players.push(newPlayers[i]);
}
emit RaffleEnter(newPlayers);
}

Remember to reset the isPlayer mapping for each player in the selectWinner function after the winner is selected and the players array is deleted.

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!