Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

A griefer could feed a very long list of addresses to enterRaffle and waste a lot of gas leading to a denial of service

Summary

A griefer could feed a very long list of addresses into enterRaffle to grief the protocol due to all the for loops in this function. Granted a griefer would have to send ether for the entrance fees for a large number of entries but he could put two duplicates in his list to guarantee that the whole function would ultimately revert if it didn't run out of gas.

Vulnerability Details

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);
}

Impact

Gas would be wasted or depleted.

Tools Used

Manual review

Recommendations

Place a limitation on the number of entries someone can get with one call to enterRaffle. Then you can also use unchecked for the i++ and j++ in the for loops. Add the following check at the beginning of the function...you could parameterize the limit on entries and allow it to be changed by the contract owner instead if you want:

if(address.length > 10) {
revert PuppyRaffle__TooManyEntries()};
Updates

Lead Judging Commences

patrickalphac Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Gas optimizations

Support

FAQs

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