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

The `enterRaffle()` function checks for duplicate players looping through the players array which can result in DoS attack

Summary

The duplicate check in the PuppyRaffle::enterRaffle function is gas expensive and can cause denial of service attack, making the subsiquent participants to spend more gas to enter the raffle.

Vulnerability Details

In the enterRaffle() function, the newPlayers array is iterated over to add each player to the players array. Then, the function checks for duplicates by iterating over all pairs of players. If a user enters the raffle with a large number of unique addresses, this could consume a lot of gas, leading to denial of service and making the function too expensive to call for anyone else.

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
//@audit DoS attack vector
@> 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

A malicious user can potentially halt the raffle by exploiting the enterRaffle() function. This function allows a user to enter the raffle multiple times, as long as they provide a unique address for each entry. The function does not check if a user has already entered the raffle, so a malicious user could enter the raffle with a large number of unique addresses, which would consume a lot of gas and potentially make the function too expensive to call for anyone else.
To demonstrate this vulnerability we can add the following test function to the file PuppyRaffleTest.t.sol and execute it using Foundry.

function testDuplicatesGasCost() public {
vm.txGasPrice(1);
//enter 100 players into the raffle
uint256 playersNum = 100;
address[] memory players = new address[](playersNum);
for(uint256 i = 0; i < playersNum; i++) {
players[i] = address(i);
}
uint256 gasStart = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNum}(players);
uint256 gasEnd = gasleft();
uint256 gasUsedFirst = (gasStart - gasEnd) * tx.gasprice;
console.log("Gas used for the first 100 players", gasUsedFirst);
// enter the next 100 players
for(uint256 i = 0; i < playersNum; i++) {
players[i] = address(i + playersNum);
}
gasStart = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNum}(players);
gasEnd = gasleft();
uint256 gasUsedSecond = (gasStart - gasEnd) * tx.gasprice;
console.log("Gas used for the second 100 players", gasUsedSecond);
assert(gasUsedFirst < gasUsedSecond);
}

The testDuplicatesGasCost() function shows the gas used for the first 100 players which entered the raffle amd the second 100 players.
The result is:

Logs:
Gas used for the first 100 players 6252039
Gas used for the second 100 players 18067748

The second 100 players have to pay significantlly more gas to enter the raffle, making the raffle unfair.

Tools Used

VS Code, Foundry

Recommendations

You can use a mapping to check duplicates. You can store a uint256 id for each raffle and the mapping would be a player address mapped to the raffle id and check with one loop if this raffleId already exists:

+ mapping(address => uint256) public addressToRaffleId;
+ uint256 public raffleId = 0;
// .........
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]);
+ addressToRaffleId[newPlayers[i]] = raffleId;
}
for (uint256 i = 0; i < players.length - 1; i++) {
+ require(addressToRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
- for (uint256 j = i + 1; j < players.length; j++) {
- require(players[i] != players[j], "PuppyRaffle: Duplicate player");
- }
}
emit RaffleEnter(newPlayers);
}
// ...
function selectWinner() external {
+ raffleId = raffleId + 1;
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
// .........
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year 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.