MyCut

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

`ContestManager::fundContest` has no "already funded" check, so funding a Pot twice locks the extra tokens

Root + Impact

fundContest transfers totalRewards tokens to the Pot every time it is called, and nothing records that the Pot was already funded. If the owner calls it twice for the same contest (for example, after a retry or a double click), the Pot holds twice the promised amount. The Pot only pays out based on remainingRewards, so the extra tokens can never be paid out and stay locked.

Description

function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
token.transferFrom(msg.sender, address(pot), totalRewards); // no check if the Pot is already funded
}

claimCut and closePot pay out only from remainingRewards, which is set once to totalRewards in the constructor. Neither Pot nor ContestManager has a function that can send the extra tokens back.

Risk

Likelihood: Low

Only the owner can call fundContest, so the owner has to fund the same contest twice by mistake.
Nothing in the code warns about it.

Impact: Medium

An amount equal to totalRewards (60 tokens in the PoC) is locked in the Pot.
Only the owner's own tokens for this one contest are affected.

Proof of Concept

Add the following inside the existing TestMyCut contract in test/TestMyCut.t.sol. It uses the existing setup, imports and the mintAndApproveTokens modifier.

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_fundContestTwice_tokensLocked() public mintAndApproveTokens {
Pot pot = _createFundedPot(10, 20, 30, 60); // funded once inside the helper
​
// the owner funds the same contest again
vm.prank(user);
ContestManager(conMan).fundContest(0);
​
console.log("tokens in the Pot:", weth.balanceOf(address(pot)));
console.log("remainingRewards :", pot.getRemainingRewards());
​
// all players claim in time
vm.prank(player1);
pot.claimCut();
vm.prank(player2);
pot.claimCut();
vm.prank(player3);
pot.claimCut();
​
vm.warp(block.timestamp + 91 days);
vm.prank(user);
ContestManager(conMan).closeContest(address(pot));
​
console.log("tokens left in the Pot after close:", weth.balanceOf(address(pot)));
​
assertEq(pot.getRemainingRewards(), 0);
assertEq(weth.balanceOf(address(pot)), 60); // the second funding is stuck
}

Run from the repository root:

forge test --mt test_PoC_fundContestTwice_tokensLocked -vv

Output:

[PASS] test_PoC_fundContestTwice_tokensLocked() (gas: 1219107)
...
tokens in the Pot: 120
remainingRewards : 60
tokens left in the Pot after close: 60

Recommended Mitigation

Remember that a Pot was funded and refuse a second funding. The Pot then always holds exactly the promised amount:

+ error ContestManager__AlreadyFunded();
+ mapping(address => bool) public isFunded;
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
+ if (isFunded[address(pot)]) revert ContestManager__AlreadyFunded();
+ isFunded[address(pot)] = true;
...
}
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!