MyCut

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

Missing re-close guard allows `closePot()` to be called multiple times, repeatedly redistributing the contract's real token balance using a stale `remainingRewards` value

Missing re-close guard allows closePot() to be called multiple times, repeatedly redistributing the contract's real token balance using a stale remainingRewards value

Severity: High

Description:

closePot() computes managerCut and claimantCut from remainingRewards, but never updates or resets remainingRewards afterward, and has no flag or check preventing it from being called more than once:

function closePot() external onlyOwner {
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) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Because remainingRewards is never decremented inside this function, every subsequent call to closePot() (once the 90-day window has passed) recomputes the same managerCut and claimantCut from the same stale value, and re-sends tokens to the owner and the same set of claimants again. This is not limited to a deliberately malicious owner — the contest README explicitly designates the Owner/Admin as a Trusted actor, so this finding does not rely on assuming bad intent. Rather, the absence of any "already closed" safeguard means this can be triggered by ordinary operational mistakes: a second admin unaware the pot was already closed, a duplicated/retried transaction, or simple human error — with the same damaging result regardless of intent.

Impact:

Repeated calls to closePot() continue transferring real tokens out of the Pot contract — to the manager and to the original claimants — until the contract's actual token balance is exhausted (at which point transfer calls begin reverting). This distributes far more of the pool than was ever intended to the manager and the first set of claimants, at the direct expense of the contract's remaining/reserved balance, and provides no protection against accidental or duplicate execution by an otherwise honest, trusted operator.

Proof of Concept:

function test_closePotCanBeCalledRepeatedlyDrainingFunds() public {
vm.warp(block.timestamp + 90 days + 1);
uint256 ownerBalanceBefore = token.balanceOf(owner);
vm.prank(owner);
pot.closePot(); // first, legitimate close
uint256 ownerBalanceAfterFirst = token.balanceOf(owner);
vm.prank(owner);
pot.closePot(); // second call — should revert or no-op, but does not
uint256 ownerBalanceAfterSecond = token.balanceOf(owner);
// Expected: ownerBalanceAfterSecond == ownerBalanceAfterFirst (no further payout possible)
// Actual: owner receives an additional managerCut, proving closePot() is not idempotent
assert(ownerBalanceAfterSecond > ownerBalanceAfterFirst);
}

Recommendation:

Add an explicit closed-state flag, set before any external calls (consistent with the checks-effects-interactions pattern already correctly used in claimCut()):

+ error Pot__AlreadyClosed();
+ bool private isPotClosed;
function closePot() external onlyOwner {
+ if (isPotClosed) {
+ revert Pot__AlreadyClosed();
+ }
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
+ isPotClosed = true;
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 24 minutes 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!