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

The Admin user can not withdraw any excessive tokens funded to a specific Pot balance due to the lack of a "Withdraw" function

aeff

Summary

The "ContestManager.sol" contract lacks a "Fund Withdraw" function which leads to loss of funds and incorrect manager and claimant cut calculation on closing the Pot.

Vulnerability Details

The "ContestManager.sol" contract has a "fundContest" function which can be used by the Admin user to fund an already created contest Pot, however the "ContestManager.sol" contract does not have any fund withdraw function to reverse this operation. That means if the Admin user sends an excessive fund by mistake to a specific Pot, this excessive fund will be stuck on the Pot, and this operation cannot be reversed.

Impact

Lack of "Fund Withdraw" function leads to the following:

1- Loss of funds as excessive funds sent to specific Pot cannot be withdrawn by the Admin uesr.

2- Incorrect calculation of the manager cut and claimant cut on closing the Pot using the Pot::closePot function which allows these actors to take more cut than they should when the Pot is closed.

Tools Used

Manual Code Review and Foundry

Recommendations

It is recommended to create a contestFundWithdraw function in the "ContestManager.sol" contract so that the Admin user can withdraw excessive funds that are sent by mistake to any create contest Pot. The proposed contestFundWithdraw function will call another created function transferFund within the "Pot.sol" contract.

An example simplified implementation can be as follows:

1- Create the following protected public and internal "contestFundWithdraw" functions in the "ContestManager.sol" contract as follows:

​
​
// New public function to withdraw fund from a specific contest Pot
function contestFundWithdraw(
address contest,
uint256 amount
) public onlyOwner {
_contestFundWithdraw(contest, amount);
}
​
// New internal function to withdraw fund from a specific contest Pot which is called by the public function contestFundWithdraw
function _contestFundWithdraw(address contest, uint256 amount) internal {
Pot pot = Pot(contest);
pot.transferFund(msg.sender, amount);
}
​
​

2- Add the following public and protected function "trasverFund" in the "Pot.sol" contract to send tokens back to the Admin user:

​
// New public function to transfer fund to the admin user who created the contest Pot
function transferFund(address admin, uint256 reward) public onlyOwner {
i_token.transfer(admin, reward);
}

Please note that the above implementation is a simplified and reduced one just for demo purposes, and further constrains should be added such as the following:

1- Add additional logic to check if a specific Pot has started by Admin user

2- Add a function to allow Admin to mark a specific Pot started

3- Admin users can not withdraw funds from a Pot using the newly implemented "contestFundWithdraw" once the Pot has been started to avoid disrupting the Pot balance and the participating players

Proof of Concept

To demonstrate and test the newly added and simplified "contestFundWithdraw" function proposed in the recommendation section, add the following test in TestMyCut.t.sol, and execute the following command:

forge test --mt testCanWithdrawPotFund -vv

function testCanWithdrawPotFund() public mintAndApproveTokens {
vm.startPrank(user);
uint256 adminBalance = ERC20Mock(weth).balanceOf(user);
console.log(
"Admin balance before funding the Pot ",
adminBalance / 1e18,
"ether"
);
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(ERC20Mock(weth)),
500 ether
);
uint256 potBalance = ERC20Mock(weth).balanceOf(contest);
console.log(
"Pot balance before being funded by the Admin ",
potBalance / 1e18,
"ether"
);
​
ContestManager(conMan).fundContest(0);
adminBalance = ERC20Mock(weth).balanceOf(user);
console.log(
"Admin balance after funding the Pot ",
adminBalance / 1e18,
"ether"
);
assertEq(adminBalance, 500 ether);
potBalance = ERC20Mock(weth).balanceOf(contest);
console.log(
"Pot balance after being funded by the Admin ",
potBalance / 1e18,
"ether"
);
assertEq(potBalance, 500 ether);
console.log(
"#########################################################"
);
console.log(
"Admin executes the new contestFundWithdraw function to send a specific fund back to the admin"
);
ContestManager(conMan).contestFundWithdraw(contest, 500 ether);
adminBalance = ERC20Mock(weth).balanceOf(user);
console.log(
"Admin balance after executing the fund withdraw function",
adminBalance / 1e18,
"ether"
);
assertEq(adminBalance, 1000 ether);
potBalance = ERC20Mock(weth).balanceOf(contest);
console.log(
"Pot balance after executing the fund withdraw function",
potBalance / 1e18,
"ether"
);
assertEq(potBalance, 0);
vm.stopPrank();
}
Updates

Lead Judging Commences

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

Owner's cut is stuck in ContestManager

Appeal created

aeff Submitter
about 1 year ago
equious Lead Judge
about 1 year ago
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.