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

looping a dynamic array poses unfair gas execution to future participants

yum_cutty

Root + Impact

Description

PuppyRaffle::enterRaffle contains double for-loop that checks to ensures the newPlayer should only be able to participate once during raffle events

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

these loops involve a checks to ensures no participants should participate again in a single events, and revert if duplicate participants found. this validation has no issue.

however, the way it validates the duplication, by having double for-loop on a dynamic array PuppyRaffle::players, causes the participants who joined at later time after the protocol has lots of participants, will suffer more gas price than the one who joined early when there are verry little to no participants.

this is because the players array stores all of the participants, the more participants have joined this event, the larger the players array is, the more iteration it takes to complete the double for-loop that checks the duplicate participants, hence more gas execution will cost.

Risk

Likelihood: Medium

  • this require some amounts of participants to participate the raffle event

Impact: High/Medium

  • the later the user join, and the more participants the raffle event currently have, the higher the gas price user will suffer.

Attack Path:

  1. First 100 participants have joined the raffle event, gas consumption is recorded.

  2. Second 100 participants have joined, gas consumption is recorded.

  3. Third 100 participants has joined, gas consumption is recorded.

  4. 3 recorded gas consumption is compared

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract PuppyRaffleTest is Test {
​
PuppyRaffle puppyRaffle;
​
uint256 entranceFee = 1e18;
address feeAddress = address(99);
uint256 duration = 1 days;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(
entranceFee,
feeAddress,
duration
);
deal(address(this), 1000000000 ether);
}
​
function test_unfair_gasPrice_for_futureParticipant() public {
// prepare first 100 participants
address[] memory participants = new address[](100);
for (uint256 i = 0; i < 100; i++) {
participants[i] = address(uint160(i + 1));
}
​
// first 100th enters
uint gasBefore = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * participants.length}(participants);
uint gasAfter = gasleft();
​
console.log("Total Gas for 1st 100 Participants:", gasBefore - gasAfter);
​
// prepare second 100 participants
for (uint256 i = 0; i < 100; i++) {
participants[i] = address(uint160(i + 101));
}
​
// second 100th enters
gasBefore = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * participants.length}(participants);
gasAfter = gasleft();
​
console.log("Total Gas for 2nd 100 Participants:", gasBefore - gasAfter);
​
// prepare third 100 participants
for (uint256 i = 0; i < 100; i++) {
participants[i] = address(uint160(i + 201));
}
// third 100th enters
gasBefore = gasleft();
puppyRaffle.enterRaffle{value: entranceFee * participants.length}(participants);
gasAfter = gasleft();
​
console.log("Total Gas for 3rd 100 Participants:", gasBefore - gasAfter);
}
​
}

create a new test file, paste the above PoC into the newly created test file, and run it.

below is the test log result:

Logs:
Total Gas for 1st 100 Participants: 6523175
Total Gas for 2nd 100 Participants: 18995134
Total Gas for 3rd 100 Participants: 39835835

the log above clearly shown that user who joined at later time during the event that has already been lots of participants will suffer more gas price than the user who joined during the time when the event has very little to no participants.

Recommended Mitigation

consider using EnumerableSet library by OpenZeppelin as an alternative to the dynamic loop PuppyRaffle::particiants

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 6 hours 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!