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

Owner Cut Stuck in `ContestManager`

abhishekthakur

Summary

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
ContestManager

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:

    contract Pot is Ownable(msg.sender) { ... }

  2. Contract Creation Context:
    The ContestManager contract creates new Pot instances through its createContest function:

    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:

    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

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:

[⠊] 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

Tools Used

Manual Review, Foundry

Recommendations

Add a claimERC20 function ContestManager to solve this issue.

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'
);
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Owner's cut is stuck in ContestManager

Support

FAQs

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