MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Gas Limit DoS via large amount of claimants

Hoover

Summary

The Pot.sol contract contains a vulnerability that can lead to a Denial of Service (DoS) attack. This issue arises from the inefficient handling of claimants in the closePot function, where iterating over a large number of claimants can cause the transaction to run out of gas, thereby preventing the contract from executing as intended.

Vulnerability Details

Affected code - https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/Pot.sol#L58

The vulnerability is located in the closePot function of the Pot contract, specifically at the loop iterating over the claimants array:

function closePot() external onlyOwner {
...
if (remainingRewards > 0) {
...
@> for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

The closePot function is designed to distribute remaining rewards to claimants after a contest ends. However, if the number of claimants is extremly large, the loop iterating over the claimants array can consume a significant amount of gas. This can lead to a situation where the transaction exceeds the gas limit and fails, effectively making it impossible to close the pot and distribute the rewards.

Exploit

  1. Attacker initiates a big contest with a lot of players

  2. People claim the cut

  3. Owner closes the large pot that will be very costly

function testGasCostForClosingPotWithManyClaimants() public mintAndApproveTokens {
// Generate 2000 players
address[] memory players2000 = new address[](2000);
uint256[] memory rewards2000 = new uint256[](2000);
for (uint256 i = 0; i < 2000; i++) {
players2000[i] = address(uint160(i + 1));
rewards2000[i] = 1 ether;
}
​
// Create a contest with 2000 players
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players2000, rewards2000, IERC20(ERC20Mock(weth)), 2000 ether);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
// Allow 1500 players to claim their cut
for (uint256 i = 0; i < 1500; i++) {
vm.startPrank(players2000[i]);
Pot(contest).claimCut();
vm.stopPrank();
}
​
// Fast forward time to allow closing the pot
vm.warp(91 days);
​
// Record gas usage for closing the pot
vm.startPrank(user);
uint256 gasBeforeClose = gasleft();
ContestManager(conMan).closeContest(contest);
uint256 gasUsedClose = gasBeforeClose - gasleft();
vm.stopPrank();
​
console.log("Gas used for closing pot with 1500 claimants:", gasUsedClose);
}
Gas used for closing pot with 1500 claimants: 6425853

Impact

The primary impact of this vulnerability is a Denial of Service (DoS) attack vector. An attacker (or even normal usage with a large number of claimants) can cause the closePot function to fail due to excessive gas consumption. This prevents the distribution of remaining rewards and the execution of any subsequent logic in the function, potentially locking funds in the contract indefinitely. In the case of smaller pots it would be a gas inefficency to itterate over the state variabel claimants.

Tools Used

Manual Review and Forge Tests

Recommendations

Gas Optimization: Optimize the loop to reduce gas consumption by using a local variable to itterate over, like in the following example:

- for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
- }
+ uint256 claimants_length = claimants.length;
+ ...
+ for (uint256 i = 0; i < claimants_length; i++) {
+ _transferReward(claimants[i], claimantCut);
+ }

Batch Processing: Implement batch processing for distributing rewards. This will redesign the protocol functionallity but instead of processing all claimants in a single transaction, allow the function to process a subset of claimants per transaction. This can be achieved by introducing pagination or limiting the number of claimants processed in one call. This could also be fixed if the user would claim their reward after 90 days themselves

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Unbound loop in closePot

Appeal created

14xSachet Auditor
about 1 year ago
equious Lead Judge
about 1 year ago
14xSachet Auditor
about 1 year ago
14xSachet Auditor
about 1 year ago
equious Lead Judge
about 1 year ago
equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Unbound loop in closePot

Support

FAQs

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