Submission Details
Severity:
**[H-2] `fundContest` allows closed Pots to be funded**
**Description:** The `fundContest` function does not check if a particular Pot / Contest has been closed.
```solidity
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();
}
// audit: q - Assumes the msg.sender has approved the ContestManager to spend funds.
token.transferFrom(msg.sender, address(pot), totalRewards);
}
```
This can result in funds being sent to a closed pot if the `index` parameter used corresponds to an contest `address` that has been closed.
**Impact:** Funds sent to a closed pot will not be redeemable again by contestants, but only extractable by existing claimaints and the manager, when the manager decides to close the pot again. This could occur out of mistake given we fund via `index` and not an `address` directly. Fat finger error could lead to the incorrect or a closed contest being funded.
**Proof of Concept:**
Add the following function to the `TestMyCut.t.sol:TestMyCut` test contract suite.
```solidity
/**
Demonstrating a closed pot being funded by the contest manager, resulting in unclaimable funds by the users, however funds can only be claimed when the owner re-closes the pot.
*/
function testCanFundClosedContestAndClaimReverts() public mintAndApproveTokens {
// Arrange
vm.startPrank(user); // Owner of contest
ContestManager contestManager = ContestManager(conMan);
uint256 userInitialBalance = weth.balanceOf(user);
console.log("User initial balance %d", userInitialBalance);
contest = contestManager.createContest(players, rewards, weth, totalRewards);
contestManager.fundContest(0);
vm.stopPrank();
// Let users claim
Pot contestPot = Pot(contest);
for (uint256 i = 0; i < players.length; i++) {
vm.prank(players[i]);
contestPot.claimCut(); // Cut has been claimed by users.
}
vm.warp(91 days);
vm.prank(conMan);
contestPot.closePot();
uint256 playerOneInitialBalance = weth.balanceOf(player1);
uint256 playerTwoInitialBalance = weth.balanceOf(player2);
// Act - Here we can fund the closedPot contest again and users cannot claim.
vm.prank(user);
contestManager.fundContest(0); // We shouldn't allow funding a closed pot.
for (uint256 i = 0; i < players.length; i++) {
vm.expectRevert(Pot.Pot__RewardNotFound.selector);
vm.prank(players[i]);
contestPot.claimCut(); // Will revert as reward for each user is 0 as they have claimed.
}
// Assert
uint256 playerOneSecondClaimBalance = weth.balanceOf(player1);
uint256 playerTwoSecondClaimBalance = weth.balanceOf(player2);
// Owner has funded contest twice, however users could only successfully claim once via claimCut.
assert(playerOneInitialBalance == playerOneSecondClaimBalance);
assert(playerTwoInitialBalance == playerTwoSecondClaimBalance);
assert(userInitialBalance - weth.balanceOf(user) == 2 * totalRewards);
}
```
**Recommended Mitigation:** `fundContest` checks the `contestToTotalRewards` mapping for the `totalRewards` value, however if this value was zero, then the attempt to fund the contest will revert. This suggests we should ensure that when we invoke `closeContest` we also remove the `contestToTotalRewards` entry for the contest address we are closing.
```diff
function _closeContest(address contest) internal {
+ delete contestToTotalRewards[contest];
Pot pot = Pot(contest);
pot.closePot();
}
```
Rewards breakdown
nSLOC
106This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.