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]);
}
@> 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);
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);
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");
// .........
}