Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Looping through player array to check for duplicates in the `PupplyRaffle::enterRaffle` is a potencial denial of servide (DoS) attack, incrementing gas cost for future entrants.

gush

Root + Impact

Description

  • The `PupplyRaffle::enterRaffle` funciton loops through the `PupplyRaffle::players` array to check for duplicates. However, the longer the `PupplyRaffle::players` is the more check a new player will have to make. This means that the gas cost for players who entry early will be dramatically lower than those who enter late. Every aditional player entry in the array increase the gas cost of the transaction.

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

  • When we have 2 sets of 100 players enter, the gas cost will be such:


    - 1st 100 players: ~6503265 gas.

    - 2nd 100 players: ~18995512 gas.


    This is more than 3x time expensive for the second 100 players set.

Impact:

  • The gas cost for raffle entrants will greatly increase as more players enter the raffle. Discouragin later users from entering, and causing a rush at the start of the raffle to be one of the first entrants in the queue.


    An attacker might make the `PupplyRaffle::players` array so big, that no one else enters, guarenteeing themselves the win.

Proof of Concept

Place the following test into `PuppyRaffleTest.t.sol`.
​
```solidity
function test_dnialOfService() public {
// Let's enter 100 players
uint256 playersNumber = 100;
address[] memory players = new address[](playersNumber);
for (uint256 i = 0; i < playersNumber; i++) {
players[i] = address(i);
}
// Set a non-zero gas price for this test
uint256 customGasPrice = 1 gwei;
vm.txGasPrice(customGasPrice);
// see how much gas it takes to enter 100 players
uint256 gasStart = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNumber}(players);
uint256 gasEnd = gasleft();
uint256 gasUsed = gasStart - gasEnd;
uint256 gasCost = gasUsed * tx.gasprice;
console.log("Gas cost for 100 players:", gasCost / 1e9);
​
// Now for the second 100 players
address[] memory playersTwo = new address[](playersNumber);
for (uint256 i = 0; i < playersNumber; i++) {
playersTwo[i] = address(i + playersNumber);
}
// see how much gas it takes to enter 100 players
uint256 gasStartTwo = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * playersNumber}(playersTwo);
uint256 gasEndTwo = gasleft();
uint256 gasUsedTwo = gasStartTwo - gasEndTwo;
uint256 gasCostTwo = gasUsedTwo * tx.gasprice;
console.log("Gas used for sencond 100 players:", gasCostTwo / 1e9);
}
```

Recommended Mitigation

There are few recommendations.
​
1. Consider allowing duplicates. Users can make new wallet addresses anyways, so a duplicate check doesn't prevent the same person from entering multiple times, only the same wallet address.
2. Consider using a mapping to check for duplicates. This would allow constant time lookup of whether a user has already entered.
​
```diff
+ uint256 public raffleId;
+ mapping(address => uint256) public addressToRaffleId;
​
constructor(uint256 _entranceFee, address _feeAddress, uint256 _raffleDuration) ERC721("Puppy Raffle", "PR") {
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
​
rarityToUri[COMMON_RARITY] = commonImageUri;
rarityToUri[RARE_RARITY] = rareImageUri;
rarityToUri[LEGENDARY_RARITY] = legendaryImageUri;
​
rarityToName[COMMON_RARITY] = COMMON;
rarityToName[RARE_RARITY] = RARE;
rarityToName[LEGENDARY_RARITY] = LEGENDARY;
​
+ raffleId = 1;
}
​
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");
- }
+ require(addressToRaffleId[newPlayers[i]] != raffleId, "PuppyRaffle: Duplicate player");
+ players.push(newPlayers[i]);
}
}
​
function selectWinner() external {
@@
+ raffleId++;
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 2 days ago
Submission Judgement Published
Validated
Assigned finding tags:

[M-01] `PuppyRaffle: enterRaffle` Use of gas extensive duplicate check leads to Denial of Service, making subsequent participants to spend much more gas than prev ones to enter

## Description `enterRaffle` function uses gas inefficient duplicate check that causes leads to Denial of Service, making subsequent participants to spend much more gas than previous users to enter. ## Vulnerability Details In the `enterRaffle` function, to check duplicates, it loops through the `players` array. As the `player` array grows, it will make more checks, which leads the later user to pay more gas than the earlier one. More users in the Raffle, more checks a user have to make leads to pay more gas. ## Impact As the arrays grows significantly over time, it will make the function unusable due to block gas limit. This is not a fair approach and lead to bad user experience. ## POC In existing test suit, add this test to see the difference b/w gas for users. once added run `forge test --match-test testEnterRaffleIsGasInefficient -vvvvv` in terminal. you will be able to see logs in terminal. ```solidity function testEnterRaffleIsGasInefficient() public { vm.startPrank(owner); vm.txGasPrice(1); /// First we enter 100 participants uint256 firstBatch = 100; address[] memory firstBatchPlayers = new address[](firstBatch); for(uint256 i = 0; i < firstBatchPlayers; i++) { firstBatch[i] = address(i); } uint256 gasStart = gasleft(); puppyRaffle.enterRaffle{value: entranceFee * firstBatch}(firstBatchPlayers); uint256 gasEnd = gasleft(); uint256 gasUsedForFirstBatch = (gasStart - gasEnd) * txPrice; console.log("Gas cost of the first 100 partipants is:", gasUsedForFirstBatch); /// Now we enter 100 more participants uint256 secondBatch = 200; address[] memory secondBatchPlayers = new address[](secondBatch); for(uint256 i = 100; i < secondBatchPlayers; i++) { secondBatch[i] = address(i); } gasStart = gasleft(); puppyRaffle.enterRaffle{value: entranceFee * secondBatch}(secondBatchPlayers); gasEnd = gasleft(); uint256 gasUsedForSecondBatch = (gasStart - gasEnd) * txPrice; console.log("Gas cost of the next 100 participant is:", gasUsedForSecondBatch); vm.stopPrank(owner); } ``` ## Recommendations Here are some of recommendations, any one of that can be used to mitigate this risk. 1. User a mapping to check duplicates. For this approach you to declare a variable `uint256 raffleID`, that way each raffle will have unique id. Add a mapping from player address to raffle id to keep of users for particular round. ```diff + uint256 public raffleID; + mapping (address => uint256) public usersToRaffleId; . . 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]); + usersToRaffleId[newPlayers[i]] = true; } // Check for duplicates + for (uint256 i = 0; i < newPlayers.length; i++){ + require(usersToRaffleId[i] != raffleID, "PuppyRaffle: Already a participant"); - 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); } . . . function selectWinner() external { //Existing code + raffleID = raffleID + 1; } ``` 2. Allow duplicates participants, As technically you can't stop people participants more than once. As players can use new address to enter. ```solidity 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]); } emit RaffleEnter(newPlayers); } ```

Support

FAQs

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

Give us feedback!