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

Incorrect Transfer of Manager Cut in `Pot::closePot`

esoetheric

Summary

In the Pot::closePot function, the manager cut is transferred to the msg.sender, which in this case is the ContestManager contract itself, not the owner. However, the ContestManager contract does not have a withdraw function, effectively locking the manager's cut in the contract without a way to retrieve it.

Impact

The manager cut is irretrievably locked in the contract, and the manager (owner) cannot access these funds since there is no withdraw function in ContestManager.

Tools Used

Manual Review.

Proof of Concept

The following variables should be added to TestMyCut.t.sol:

address[] testPlayers = [
address(uint160(1)),
address(uint160(2)),
address(uint160(3)),
address(uint160(4)),
address(uint160(5)),
address(uint160(6)),
address(uint160(7)),
address(uint160(8)),
address(uint160(9)),
address(uint160(10))
];
uint256[] testRewards = [
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether
];
uint256 testTotalRewards = 20 ether;

Then, add the following test to the existing tests in TestMyCut.t.sol:

function testLockedManagerCut() public mintAndApproveTokens {
vm.startPrank(user);
ContestManager testConMan = ContestManager(conMan);
​
address testPot = testConMan.createContest(
testPlayers,
testRewards,
IERC20(weth),
testTotalRewards
);
testConMan.fundContest(0);
vm.stopPrank();
uint256 numberOfClaimers = 5;
for (uint i; i < numberOfClaimers; i++) {
vm.prank(testPlayers[i]);
Pot(testPot).claimCut();
}
vm.warp(block.timestamp + 100 days);
console.log(
"Owner Balance before Closure: ",
weth.balanceOf(address(user))
);
uint256 ownerBalanceBeforeClosure = weth.balanceOf(address(user));
​
console.log(
"ContestManager Balance before Closure: ",
weth.balanceOf(address(conMan))
);
vm.prank(user);
testConMan.closeContest(testPot);
console.log(
"Owner Balance after Closure: ",
weth.balanceOf(address(user))
);
console.log(
"ContestManager Balance after Closure: ",
weth.balanceOf(address(conMan))
);
assertNotEq(ownerBalanceBeforeClosure, weth.balanceOf(address(user)));
}

Finally run the tests with:

forge test

Recommended Mitigation

Consider adding a withdraw function to ContestManager.sol or directly transferring the funds to owner.
For instance a function like the following one could be added to ContestManager.sol:

function ownerWithdrawal(address token, uint256 amount) public onlyOwner {
IERC20(token).transfer(msg.sender, amount);
}
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.