MyCut

AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Impact: medium
Likelihood: low
Invalid

closePot() Can Be Called Multiple Times

Root + Impact

Description

  • The `closePot()` function can be called multiple times after the 90-day period has elapsed. While the function checks if `remainingRewards > 0` before executing, there's no explicit state flag to prevent multiple calls. If `remainingRewards` is 0 (all rewards claimed), the function will execute the time check but do nothing. However, there's no protection against calling it repeatedly, which wastes gas and could indicate a design issue.

    Additionally, if there are any edge cases where `remainingRewards` becomes non-zero again (though unlikely with current logic), the function could execute multiple times.

```solidity
// Root cause in the codebase
49| function closePot() external onlyOwner {
50| if (block.timestamp - i_deployedAt < 90 days) {
51| revert Pot__StillOpenForClaim();
52| }
53| if (remainingRewards > 0) {
54| uint256 managerCut = remainingRewards / managerCutPercent;
55| i_token.transfer(msg.sender, managerCut);
56|
57| uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
58| for (uint256 i = 0; i < claimants.length; i++) {
59| _transferReward(claimants[i], claimantCut);
60| }
61| }
62| }
```

Risk

Likelihood:

  • * This occurs when the owner calls `closePot()` multiple times after the 90-day period

    * The issue manifests every time the function is called after the initial closure

Impact:

  • * Unnecessary gas consumption from repeated calls

    * No explicit state tracking of whether the pot has been closed

    * Potential confusion about pot state

    * If logic changes in the future, multiple calls could have unintended consequences

Proof of Concept

```solidity
// Scenario: Pot closed after 90 days
// 1. Owner calls closePot() - executes successfully
// 2. Owner calls closePot() again - time check passes, remainingRewards = 0, does nothing
// 3. Owner calls closePot() again - same result, wastes gas
// No error or state indication that pot is already closed
```

Recommended Mitigation

```diff
address[] private claimants;
uint256 private immutable i_totalRewards;
uint256 private immutable i_deployedAt;
IERC20 private immutable i_token;
mapping(address => uint256) private playersToRewards;
uint256 private remainingRewards;
uint256 private constant managerCutPercent = 10;
+ bool private isClosed;
function closePot() external onlyOwner {
+ require(!isClosed, "Pot already closed");
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
+ remainingRewards = 0;
}
+ isClosed = true;
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!