Root cause: closePot finalises nothing, it neither zeroes remainingRewards nor records that the pot is closed. Impact: the whole distribution re-executes on every repeat call and drains the pot into an address nobody can withdraw from

Description

Closing a pot is meant to be the last thing that happens to it. The README describes the owner as someone who closes "old Pots when the claim period has elapsed", a one time settlement after which the contest is finished.

closePot performs the settlement but never writes down that it happened. remainingRewards keeps its pre close value, no closed flag exists, and the time guard only compares against i_deployedAt, which stays true forever once 90 days have passed. Nothing in the function or in ContestManager prevents the same call from running again. Each repeat recomputes both cuts from the same stale figure and moves another round of tokens out.

The stale number is not a display issue. It is the input the cut is computed from, so after the first close the contract takes ten percent of a balance it no longer holds, and ContestManager.getContestRemainingRewards reports the same fiction to anything integrating with it.

Risk

Likelihood:

• The call is permanently available once the window elapses, and nothing marks the pot as settled, so an ordinary retry after a dropped or reverted transaction re-runs the full distribution.

• Any operator script that closes pots on a schedule repeats it every time it runs, with no on chain guard to stop it.

Impact:

• Repeating the call on a pot where nobody claimed moves the entire balance out in ten rounds. The players receive nothing and the tokens land on ContestManager, which has no function that sends ERC20 anywhere.

• It also fires with real claimants present. With ten players and one claimant the call succeeds five times and 450 of 900 tokens are skimmed away.

Proof of Concept

Two players are registered for 500 each and nobody claims. This keeps the finding independent of how the leftover is divided, because with an empty claimants array the payout loop never executes and the computed share is discarded. The manager is the only actor and he calls the documented close function repeatedly. There is no attacker. Run with forge test --match-path test/F4_Deep.t.sol -vv.

Output:

A control test pins the defect to the missing write rather than to the close flow itself: when every player has claimed, remainingRewards is genuinely zero, the guard holds and repeat calls are harmless.

To rule out overlap with the divisor issue in the same function, I ran both fixes separately. Correcting the divisor alone changes nothing here: a variant dividing by claimants.length still replays ten times and still drains to zero, since the replay never depends on the payout loop. Zeroing the state instead stops the replay but leaves the underpayment. Neither fix removes the other, so these are separate roots.

Recommended Mitigation

Settle the accounting inside the same call that pays out, and refuse a second run:

Zeroing the field also keeps getRemainingRewards honest, so the value the factory exposes matches what the pot actually holds.