Root + Impact:
claimCut() only checks whether the caller has a non-zero reward.
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}
There is no check that the call happens within the 90-day claim period, and there is no closed-state variable that prevents claims after closePot() has already been executed.
Description:
The README states that authorized claimants have 90 days to claim. After that, the manager takes a cut of the remaining pool and the rest should be distributed equally to users who claimed in time.
However, claimCut() does not enforce this deadline. Any authorized player who did not claim during the 90-day period can still call claimCut() later.
This breaks the intended reward model because users who missed the deadline should not be able to receive their original allocation after the pot is closed.
This also interacts badly with the closePot() accounting bug: after the pot is closed, some undistributed tokens may remain stuck in the contract. Late claimants can then claim from those leftover funds, even though they were supposed to be excluded from post-deadline rewards.
Risk:
Likelihood is high because any authorized player who misses the deadline can simply call claimCut() later.
Impact is medium because late users receive rewards they should have forfeited, reducing or draining funds that should have gone to the manager and timely claimants.
Proof of Concept:
Assume:
Players: Alice, Bob
Reward per player: 100 tokens
Total rewards: 200 tokens
Claim period: 90 days
Flow:
1. Alice claims during the 90-day claim period.
2. Bob does not claim.
3. 91 days pass.
4. Owner closes the pot.
5. Bob calls claimCut() after close.
6. Bob still receives his original 100-token allocation.
Expected behavior:
Bob missed the deadline, so Bob should not be able to claim.
The remaining pool should be used for the manager cut and redistribution to timely claimants.
Actual behavior:
Bob can still claim because claimCut() only checks playersToRewards[Bob] > 0.
Foundry-style PoC:
function testPlayerCanClaimAfterDeadlineAndAfterClose() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100;
rewards[1] = 100;
uint256 totalRewards = 200;
address contest = contestManager.createContest(
players,
rewards,
IERC20(weth),
totalRewards
);
ERC20Mock(weth).approve(address(contestManager), totalRewards);
contestManager.fundContest(0);
vm.prank(alice);
Pot(contest).claimCut();
vm.warp(block.timestamp + 91 days);
contestManager.closeContest(contest);
uint256 bobBalanceBefore = ERC20Mock(weth).balanceOf(bob);
vm.prank(bob);
Pot(contest).claimCut();
uint256 bobBalanceAfter = ERC20Mock(weth).balanceOf(bob);
assertEq(bobBalanceAfter - bobBalanceBefore, 100);
}
Recommended Mitigation:
Add claim deadline enforcement to claimCut() and add a closed-state variable to prevent claims after closePot().
+ error Pot__ClaimPeriodEnded();
+ error Pot__AlreadyClosed();
+ bool private closed;
function claimCut() public {
+ if (closed) revert Pot__AlreadyClosed();
+ if (block.timestamp - i_deployedAt >= 90 days) {
+ revert Pot__ClaimPeriodEnded();
+ }
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
+ if (closed) revert Pot__AlreadyClosed();
+ closed = true;
if (remainingRewards > 0) {
...
}
}
This enforces the intended lifecycle:
Before 90 days: authorized players can claim
After 90 days: players can no longer claim
After close: no further claims are allowed
