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

duplicate entrants loop check - DOS vulnerability

Summary

If the players array will become big enough so the gas limit is not enough to calculate the tx, - it will be reverted. That would mean a denial of service, no one will be able to use the contract any more!

Vulnerability Details

Each opcode in EVM requires gas to process calculations. The gas limit is 30mil for a tx, so if the calculations will demand more, the tx will be reverted. So when there will be "too much" players, the contract will become unreachable forever...

Impact

The contract won't be available any more, so all the assets will be lost

Tools Used

hardhat

Recommendations

Instead of the array, developers need to use a mapping to store current players, it will make everything much easier

- address[] public players;
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);
}
+ mapping(address => bool) public players;
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++) {
+ if(!players[newPlayers[i]]){
+ players[newPlayers[i]] = true;
+ }
+ }
emit RaffleEnter(newPlayers);
}
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!