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

External ERC20 transfer calls inside a loop in closePot() allow a single failing recipient to permanently block fund distribution for all claimants

squashingbugs007

Root + Impact

Description

  • Normal behavior: When closePot() is called after 90 days, it distributes the remaining pool to all claimants in a single transaction by looping through the claimants array and transferring each their share.

  • The issue: Every iteration of the loop makes an external ERC20 transfer() call. This creates two distinct attack vectors.

  • First: a griefing DoS: if any single address in the claimants array is a smart contract that reverts on token receipt (via a custom receive() or token hook), that one address causes the entire closePot() call to revert. Since closePot() is the only distribution mechanism and can only be called once the 90 day window closes, one malicious or misconfigured claimant permanently blocks every other claimant from receiving their redistribution share. The owner cannot skip the bad address, the loop has no error handling.

  • Second: gas exhaustion DoS: if the claimants array grows large enough, the total gas cost of all external calls in the loop exceeds the block gas limit. closePot() becomes permanently uncallable. All remaining funds are locked in the contract forever with no alternative distribution path.

  • Both scenarios result in honest claimants being denied their rightful redistribution share through no fault of their own.

// Pot.sol lines 57-59
// @> External call inside loop — one failure blocks entire distribution
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut); // @> external ERC20 call per iteration
}
​
// Pot.sol lines 64-66
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // @> external call
}

Risk

Likelihood:

  • A malicious actor only needs to call claimCut() once with a contract address that reverts on token receipt to trigger the griefing attack.

  • This requires no special privileges claimCut() is open to any enrolled player address.

  • Gas exhaustion becomes relevant in contests with large numbers of participants all claiming before the deadline.

Impact:

  • All claimants are denied their redistribution share permanently closePot() has no retry, no skip, and no alternative path.

  • Funds remain locked in the contract with no recovery mechanism.

  • A single attacker with minimal cost (one claimCut() call) can grief every other honest participant in the contest.

Proof of Concept

The following demonstrates a malicious claimant contract that reverts on token receipt, causing closePot() to revert and blocking distribution to all other claimants.

// Malicious contract that reverts when receiving tokens
contract MaliciousClaimant {
Pot public pot;
constructor(address _pot) {
pot = Pot(_pot);
}
// Called during pot.claimCut() to register as claimant
function claim() external {
pot.claimCut();
}
// ERC20 transfer hook — reverts on receipt
// Forces closePot() loop to revert on this address
function onERC20Received(address, uint256) external pure returns (bool) {
revert("I reject tokens");
}
}
​
function test_maliciousClaimantBlocksClosePot() public {
// Setup: 3 legitimate players + 1 malicious contract
MaliciousClaimant attacker = new MaliciousClaimant(address(pot));
// Legitimate players claim
vm.prank(players[0]); pot.claimCut();
vm.prank(players[1]); pot.claimCut();
vm.prank(players[2]); pot.claimCut();
// Attacker claims — registers in claimants array
attacker.claim();
// Advance past 90 day deadline
vm.warp(block.timestamp + 91 days);
// closePot() reverts — attacker's address causes token transfer to fail
// All legitimate claimants are permanently blocked
vm.expectRevert();
pot.closePot();
// players[0], players[1], players[2] receive nothing
// funds locked forever
}

Recommended Mitigation

Replace the push payment pattern (contract sends to each claimant) with a pull payment pattern (each claimant withdraws their own share). This isolates each claimant's withdrawal, one failing address cannot affect any other. This ensures one claimant's failure never affects others, gas costs are distributed across individual transactions, and no single actor can grief the entire distribution.

+ mapping(address => uint256) private claimantClosingCut;
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.safeTransfer(msg.sender, managerCut);
​
uint256 cut = (remainingRewards - managerCut) / claimants.length;
- for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], cut);
- }
+ // Store each claimant's share — let them pull individually
+ for (uint256 i = 0; i < claimants.length; i++) {
+ claimantClosingCut[claimants[i]] += cut;
+ }
}
}
​
+ // Each claimant pulls their own closing distribution
+ function withdrawClosingCut() external {
+ uint256 amount = claimantClosingCut[msg.sender];
+ require(amount > 0, "Nothing to withdraw");
+ claimantClosingCut[msg.sender] = 0;
+ i_token.safeTransfer(msg.sender, amount);
+ }
Updates

Lead Judging Commences

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

[H-04] Gas Limit DoS via large amount of claimants

## Description 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: ```javascript 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 ```javascript 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); } ``` ```Solidity 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`. ## Recommendations Gas Optimization: Optimize the loop to reduce gas consumption by using a local variable to itterate over, like in the following example: ```diff - 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

Support

FAQs

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

Give us feedback!