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);
}
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);
vm.prank(user);
ContestManager(conMan).fundContest(0);
console.log("tokens in the Pot:", weth.balanceOf(address(pot)));
console.log("remainingRewards :", pot.getRemainingRewards());
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);
}
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;
...
}