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

`ContestManager` is the owner of `Pot` , so `managerCut` goes to `ContestManager` , and the person who deployed `ContestManager` has no way getting these funds from the `ContestManager` contract and these funds are stuck here.

SoulReaper

Summary

ContestManager is the owner of Pot , so managerCut goes to ContestManager , and the person who deployed ContestManager has no way getting these funds from the ContestManager contract and these funds are stuck here.

Vulnerability Details

A person (let, Sam) deploys the ContestManager contract. Now , ContestManager deploys the Pot contract , so ContestManager is the owner of Pot . Whenever Pot::closePot is called , managerCut is sent to the owner of the pot , i.e. , ContestManager. But there is no function in ContestManager which lets it's owner (Sam) take out the funds.

Proof of Concepts

  1. Owner(of Pot , i.e. , ContestManager) creates and funds the pool

  2. Player 1 claims

  3. Deadline passes

  4. Owner calls closePot

  5. Owner(Contest Manager) gets his 10%

  6. Owner of Contest Manager (here , user) gets nothing

Place this in TestMyCut.t.sol

function test_ManagerCutGoesToContestManager() public mintAndApproveTokens{
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
uint256 userBalanceBefore = ERC20Mock(weth).balanceOf(user);
uint256 ownerBalanceBefore = ERC20Mock(weth).balanceOf(conMan);
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
uint256 userBalanceAfter = ERC20Mock(weth).balanceOf(user);
uint256 ownerBalanceAfter = ERC20Mock(weth).balanceOf(conMan);
​
assert(ownerBalanceAfter - ownerBalanceBefore == 50); // contest manager gets the manager cut
assert (userBalanceBefore == userBalanceAfter); // owner of contest manager doesn't get anything
}

Impact

Owner of ContestManager contract gets no funds and the managerCut from all contests is stuck inside ContestManager

Tools Used

Manual review , Foundry Tests

Recommendations

  1. Make functions which owner of Contest Manager can use to pull out the funds corresponding to a particular token

  • Add these functions to ContestManager.sol

function getToken(address _pot) public view returns(IERC20 token) {
Pot pot = Pot(_pot);
token = pot.getToken();
}
​
function receiveCut(IERC20 token) public onlyOwner{
token.transfer(msg.sender , token.balanceOf(address(this)));
}

  • Owner can input address of the contest in getToken() to get the token corresponding to that contest , then use receiveCut() to pull out the funds.

  1. In the Pot::closePot , instead of transferring managerCut to msg.sender (which is ContestManager) , transfer it to tx.origin (which is owner of ContestManager) . Make the following change :

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+ i_token.transfer(tx.origin, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
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.