MyCut

Description

Pot.closePot is supposed to end the 90-day claim window: the manager takes 10% of whatever is still unclaimed, and the rest is split among players who claimed on time. After that, the pot should be economically closed — no more primary-cut outflows.

It is not. closePot never sets a closed flag, never zeros playersToRewards for players who missed the window, and never writes remainingRewards. claimCut still only checks playersToRewards[msg.sender] > 0. Any listed player who sat out the claim period can call claimCut after residual settlement and pull their original mapped cut from whatever ERC20 leftover the close left in the pot.

That leftover is exactly the residual that settlement treated as unclaimed / dust. Late claimants steal it; remaining unclaimed players are left insolvent against their still-mapped cuts.

Deep Dive

claimCut is gated only on a positive mapping entry:

There is no closed check and no block.timestamp check against i_deployedAt + 90 days.

closePot is the only settlement path. After the 90-day guard it reads remainingRewards, pays the manager, and pays residual cuts to claimants — then returns. It does not flip a flag, does not clear unclaimed playersToRewards, and does not set remainingRewards to the leftover (or to zero):

Two accounting invariants break at that point:

1. remainingRewards still equals the pre-close unclaimed primary cuts, even though the ERC20 balance has already been reduced by managerCut + claimants.length * claimantCut.

2. Every unclaimed player's playersToRewards is still their original primary cut, so claimCut will happily transfer it.

The residual formula itself already leaves tokens in the pot. Residual is divided by i_players.length (all listed players), not claimants.length, and is paid only to claimants. For U unclaimed players the pot keeps roughly U * claimantCut plus integer-division dust. That leftover is what a late claimCut drains.

Production path with players [A, B, C] mapped to 100 each, pot funded with 300:

| Step | remainingRewards | pot ERC20 | notes |
|---|---|---|---|
| fund | 300 | 300 | A, B, C each mapped 100 |
| A claimCut | 200 | 200 | A received 100, A in claimants |
| closePot | 200 (unchanged) | 120 | managerCut = 200/10 = 20; claimantCut = 180/3 = 60 paid only to A |
| B claimCut | 100 | 20 | B still mapped 100; transfer succeeds |
| C claimCut | — | 20 | C still mapped 100; transfer reverts / C is insolvent |

INV-004 requires close to be economically terminal: managerCut + residual + integer-division leftover == remainingRewards, with no further primary-cut outflows. After close, remainingRewards should no longer move and playersToRewards for unclaimed players should be zero. Neither holds.

ContestManager.closeContest is a thin owner wrapper around pot.closePot() and adds no extra terminal state.

Exploitation

No special privileges beyond being a listed player who did not claim before close. The pot leftover after close must be at least that player's original mapped cut (true for equal-cut 3-player close as below; also true whenever enough players claimed in time that residual leftover ≥ one unclaimed primary cut).

1. Owner: ContestManager.createContest([A, B, C], [100, 100, 100], token, 300)

2. Owner: ContestManager.fundContest(0) — pot holds 300.

3. A: Pot.claimCut() — A receives 100. remainingRewards = 200. claimants = [A].

4. Warp block.timestamp >= i_deployedAt + 90 days.

5. Owner: ContestManager.closeContest(pot)

• managerCut = 200 / 10 = 20 → ContestManager

• claimantCut = (200 - 20) / 3 = 60 → A

• pot ERC20 leftover = 200 - 20 - 60 = 120

• remainingRewards still 200; playersToRewards[B] = 100; playersToRewards[C] = 100

1. B: Pot.claimCut() with empty calldata.

• gate passes (playersToRewards[B] == 100)

• remainingRewards 200 → 100

• pot transfers 100 to B

• pot balance 120 → 20

B extracted 100 that settlement had already treated as residual/dust for on-time claimants (and manager). C is left with a mapped cut of 100 against a pot balance of 20.

PoC (Foundry; drop into test/TestMyCut.t.sol or a sibling file):

Impact

• Theft of residual / dust. Late players extract ERC20 that closePot already allocated as manager cut remainder + residual for on-time claimants. Severity is High: this is a direct, permissionless drain of pot funds after the protocol has declared the contest settled.

• Broken terminal accounting. remainingRewards and the pot balance keep moving after close, violating the close-is-final invariant.

• Insolvency of remaining unclaimed players. After one late claimCut, later unclaimed players still have a positive playersToRewards but the pot no longer holds enough to pay them. Their claimCut will revert in _transferReward (or succeed partially under a fee-on-transfer token, which is out of stated compatibility).

• Repeatable per unclaimed player until leftover < that player's mapped cut. The first late claimant(s) win; everyone else is stuck.

Recommendation

Make close terminal. Minimum patch:

1. Add bool private closed; (or reuse a remaining-rewards-zero convention).

2. In closePot, after residual transfers: set closed = true, zero playersToRewards for every player still mapped, and set remainingRewards to the actual leftover (or 0 if leftover is also swept).

3. In claimCut, revert if closed (or if block.timestamp >= i_deployedAt + 90 days). Do not allow primary-cut transfers after the claim window.

Also consider dividing residual by claimants.length (not i_players.length) so on-time claimants receive the full remainder and the pot is not left holding a residual stash that a late claimCut can steal. That is a separate fairness bug; the closed flag is what actually stops this drain.

Proof of Concept

diff --git a/test/ClaimCutAfterClose.t.sol b/test/ClaimCutAfterClose.t.sol
new file mode 100644
index 0000000..c3e8f21
--- /dev/null
+++ b/test/ClaimCutAfterClose.t.sol
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+// Attacker: any listed player who did not claim before closePot.
+// Impact: after residual settlement, B still claimCut()s their full 100 primary cut;
+// remainingRewards 200→100, pot balance 120→20, C left insolvent (mapped 100 vs 20).
+// Run: forge test --match-test test_claimCutAfterCloseMovesRemainingRewardsAndBalance -vv
+
+import {Test, console} from "lib/forge-std/src/Test.sol";
+import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
+import {ContestManager} from "../src/ContestManager.sol";
+import {Pot} from "../src/Pot.sol";
+import {ERC20Mock} from "./ERC20Mock.sol";
+
+contract ClaimCutAfterCloseTest is Test {

• address owner = makeAddr("owner");

• address playerA = makeAddr("playerA");

• address playerB = makeAddr("playerB");

• address playerC = makeAddr("playerC");

+

• ContestManager manager;

• ERC20Mock token;

• Pot pot;

+

• function setUp() public {

• vm.startPrank(owner);

• manager = new ContestManager();

• token = new ERC20Mock("WETH", "WETH", owner, 0);

• token.mint(owner, 300);

• token.approve(address(manager), 300);

+

• address[] memory players = new address[](3);

• players[0] = playerA;

• players[1] = playerB;

• players[2] = playerC;

• uint256[] memory rewards = new uint256[](3);

• rewards[0] = 100;

• rewards[1] = 100;

• rewards[2] = 100;

+

• pot = Pot(manager.createContest(players, rewards, IERC20(token), 300));

• manager.fundContest(0);

• vm.stopPrank();

• }

+

• function test_claimCutAfterCloseMovesRemainingRewardsAndBalance() public {

• vm.prank(playerA);

• pot.claimCut();

+

• vm.warp(block.timestamp + 90 days);

• vm.prank(owner);

• manager.closeContest(address(pot));

+

• uint256 remainingAfterClose = pot.getRemainingRewards();

• uint256 potBalanceAfterClose = token.balanceOf(address(pot));

• uint256 bBalanceAfterClose = token.balanceOf(playerB);

+

• // Settlement treated leftover as residual/dust: remainingRewards never zeroed,

• // 20 manager + 60 residual to A, 120 ERC20 still in pot.

• assertEq(remainingAfterClose, 200);

• assertEq(potBalanceAfterClose, 120);

• assertEq(pot.checkCut(playerB), 100);

• assertEq(pot.checkCut(playerC), 100);

+

• // Late unclaimed player still takes their full primary cut after close.

• vm.prank(playerB);

• pot.claimCut();

+

• uint256 remainingAfterLateClaim = pot.getRemainingRewards();

• uint256 potBalanceAfterLateClaim = token.balanceOf(address(pot));

+

• console.log("remainingRewards after close:", remainingAfterClose);

• console.log("pot balance after close:", potBalanceAfterClose);

• console.log("remainingRewards after B late claim:", remainingAfterLateClaim);

• console.log("pot balance after B late claim:", potBalanceAfterLateClaim);

• console.log("B extracted:", token.balanceOf(playerB) - bBalanceAfterClose);

• console.log("C still mapped cut:", pot.checkCut(playerC));

+

• assertEq(remainingAfterLateClaim, 100);

• assertEq(potBalanceAfterLateClaim, 20);

• assertEq(token.balanceOf(playerB) - bBalanceAfterClose, 100);

• assertEq(pot.checkCut(playerB), 0);

• // C is insolvent: still mapped 100 against 20 leftover.

• assertEq(pot.checkCut(playerC), 100);

• assertLt(potBalanceAfterLateClaim, pot.checkCut(playerC));

• }

+}