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

Missing Boolean Return Check on ERC20 Transfers which may lead to potential loss of funds

dharmin

Description

In MyCut::Pot.sol, the _transferReward() function does not check the boolean return value of the transfer function. ERC20 tokens return a boolean indicating whether the transfer was successful. Failure to check this return value means that if the transfer fails, the code will continue execution without reverting, potentially leading to unintended outcomes.

Impact

If the transfer fails and the return value is not checked, the remaining rewards will not be transferred to the msg.sender, leading to a potential loss of funds. The protocol will proceed as if the transfer was successful. The lack of safe transfer functions may result in incorrect accounting and a loss of trust from users.

Proof of Concepts

In the Pot::_transferReward() function, the transfer function is not checked for success

function _transferReward(address player, uint256 reward) internal {
@> i_token.transfer(player, reward);
}

Also in the Pot::closePot(), while transferring the managerCut we are not checking for success

if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
@> i_token.transfer(msg.sender, managerCut); // No check for transfer success
// If the transfer fails, there is no revert, and the process continues
}

Also in the ContestManager::fundContest(), while funding the contest

function fundContest(uint256 index) public onlyOwner {
// ...
// ...
// ...
​
@> token.transferFrom(msg.sender, address(pot), totalRewards);
}

Recommended Mitigation: Check the boolean return value of the transfer function and revert if the transfer fails. The recommended pattern is as follows:

function _transferReward(address player, uint256 reward) internal {
bool success = i_token.transfer(player, reward);
require(success, "Transfer failed");
}

Also, in the Pot::closePot() function, it can be handled in a better way like this:

if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
bool success = i_token.transfer(msg.sender, managerCut);
require(success, "Transfer failed");
}

This ensures that the protocol reverts if the transfer fails

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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