MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

[H-4] Manager Fees Permanently Locked in ContestManager Contract

jck

[H-4] Manager Fees Permanently Locked in ContestManager Contract

Description

When contests are closed via ContestManager.closeContest(), the 10% manager fee is sent to the ContestManager contract instead of the owner. The contract has no way to withdraw these fees, causing permanent loss.

Root Cause

  1. Ownership mismatch: ContestManager becomes Pot owner when creating contests

  2. Wrong recipient: Pot.closePot() sends fees to msg.sender (ContestManager) instead of owner()

  3. No withdrawal: ContestManager lacks function to recover accumulated fees

Risk

Likelihood: High - Affects 100% of contests closed via ContestManager
Impact: High - All manager fees permanently locked, breaks business model
Severity: High (H) - Direct financial loss, no recovery possible

Proof of Concept

What the POC shows:

Setup: ContestManager creates a pot with 2 players, each owed 0.5 ETH (total 1 ETH)
Execution: Only player1 claims their 0.5 ETH, leaving 0.5 ETH unclaimed
Bug trigger: ContestManager closes the pot after 90 days
Proof: The 10% manager fee (0.05 ETH) goes to ContestManager contract instead of the owner

Mathematical proof:

Remaining rewards: 0.5 ETH
Manager fee (10%): 0.05 ETH
Expected: 0.05 ETH → ContestManager owner
Actual: 0.05 ETH → ContestManager contract (LOCKED!)
function test_ManagerFeesLocked_PoC() public {
// Create contest via ContestManager
address[] memory players = new address[](2);
uint256[] memory rewards = new uint256[](2);
players[0] = player1; rewards[0] = 0.5 ether;
players[1] = player2; rewards[1] = 0.5 ether;
​
vm.prank(user);
address potAddr = contestManager.createContest(players, rewards, weth, 1 ether);
// Fund and partially claim
vm.prank(user);
weth.approve(address(contestManager), 1 ether);
contestManager.fundContest(0);
vm.prank(player1);
Pot(potAddr).claimCut(); // Claims 0.5 ETH
// Close via ContestManager
vm.warp(block.timestamp + 91 days);
vm.prank(user);
contestManager.closeContest(potAddr);
// 0.05 ETH fee went to ContestManager, not owner
assertEq(weth.balanceOf(address(contestManager)), 0.05 ether);
// Owner received 0 ETH from fee
}

Recommended Mitigation

Explanation

  1. Correct Destination: Changing msg.sender to owner() in Pot.sol ensures fees go to the intended admin, even when a contract triggers the call.

  2. Pull Pattern: Adding withdrawFees to ContestManager.sol provides a "door" to move funds out, preventing the contract from becoming a "black hole."

  3. SafeERC20: Using safeTransfer handles non-standard tokens (like USDT) that don't return booleans, preventing silent transfer failures.

  4. Balance Sweeping: Using balanceOf(address(this)) captures 100% of funds, including rounding "dust" and accidental transfers.

// In Pot.sol:
function closePot() external onlyOwner {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+ i_token.transfer(owner(), managerCut); // Fix: Send to actual owner, not caller
}
​
// In ContestManager.sol:
+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+ using SafeERC20 for IERC20;
+
+ function withdrawFees(IERC20 token) external onlyOwner {
+ uint256 balance = token.balanceOf(address(this));
+ if (balance > 0) {
+ token.safeTransfer(owner(), balance);
+ emit FeesWithdrawn(address(token), balance);
+ }
+ }

Why This Is Critical

  • Non-recoverable: Locked tokens cannot be retrieved

  • Business-breaking: No compensation for managing contests

  • Scales linearly: Each contest adds to locked amount

  • Violates trust: Manager expects but receives no fees

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-01] Owner Cut Stuck in `ContestManager`

## Description When `closeContest` function in the `ContestManager` contract is called, `pot` sends the owner's cut to the `ContestManager` itself, with no mechanism to withdraw these funds. ## Vulnerability Details: Relevant code - [Pot](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L7) [ContestManager](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/ContestManager.sol#L16-L26) The vulnerability stems from current ownership implementation between the `Pot` and `ContestManager` contracts, leading to funds being irretrievably locked in the `ContestManager` contract. 1. **Ownership Assignment**: When a `Pot` contract is created, it assigns `msg.sender` as its owner: ```solidity contract Pot is Ownable(msg.sender) { ... } ``` 2. **Contract Creation Context**: The `ContestManager` contract creates new `Pot` instances through its `createContest` function: ```solidity function createContest(...) public onlyOwner returns (address) { Pot pot = new Pot(players, rewards, token, totalRewards); ... } ``` In this context, `msg.sender` for the new `Pot` is the `ContestManager` contract itself, not the external owner who called `createContest`. 3. **Unintended Ownership**: As a result, the `ContestManager` becomes the owner of each `Pot` contract it creates, rather than the intended external owner. 4. **Fund Lock-up**: When `closeContest` is called (after the 90-day contest period), it triggers the `closePot` function: ```solidity function closeContest(address contest) public onlyOwner { Pot(contest).closePot(); } ``` The `closePot` function sends the owner's cut to its caller. Since the caller is `ContestManager`, these funds are sent to and locked within the `ContestManager` contract. 5. **Lack of Withdrawal Mechanism**: The `ContestManager` contract does not include any functionality to withdraw or redistribute these locked funds, rendering them permanently inaccessible. This ownership misalignment and the absence of a fund recovery mechanism result in a critical vulnerability where contest rewards become permanently trapped in the `ContestManager` contract. ## POC In existing test suite, add following test ```solidity function testOwnerCutStuckInContestManager() public mintAndApproveTokens { vm.startPrank(user); contest = ContestManager(conMan).createContest( players, rewards, IERC20(ERC20Mock(weth)), 100 ); ContestManager(conMan).fundContest(0); vm.stopPrank(); // Fast forward 91 days vm.warp(block.timestamp + 91 days); uint256 conManBalanceBefore = ERC20Mock(weth).balanceOf(conMan); console.log("contest manager balance before:", conManBalanceBefore); vm.prank(user); ContestManager(conMan).closeContest(contest); uint256 conManBalanceAfter = ERC20Mock(weth).balanceOf(conMan); // Assert that the ContestManager balance has increased (owner cut is stuck) assertGt(conManBalanceAfter, conManBalanceBefore); console.log("contest manager balance after:", conManBalanceAfter); } ``` run `forge test --mt testOwnerCutStuckInContestManager -vv` in the terminal and it will return following output: ```js [⠊] Compiling... [⠑] Compiling 1 files with Solc 0.8.20 [⠘] Solc 0.8.20 finished in 1.66s Compiler run successful! Ran 1 test for test/TestMyCut.t.sol:TestMyCut [PASS] testOwnerCutStuckInContestManager() (gas: 810988) Logs: User Address: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Contest Manager Address 1: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 Minting tokens to: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Approved tokens to: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 contest manager balance before: 0 contest manager balance after: 10 Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 10.51ms (1.31ms CPU time) ``` ## Impact Loss of funds for the protocol / owner ## Recommendations Add a claimERC20 function `ContestManager` to solve this issue. ```solidity function claimStuckedERC20(address tkn, address to, uint256 amount) external onlyOwner { // bytes4(keccak256(bytes('transfer(address,uint256)'))); (bool success, bytes memory data) = tkn.call(abi.encodeWithSelector(0xa9059cbb, to, amount)); require( success && (data.length == 0 || abi.decode(data, (bool))), 'ContestManager::safeTransfer: transfer failed' ); ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!