MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

The return value of `ERC20:transfer()` and `ERC20:transferFrom()` functions in never evaluated which may lead to silent fail of token transfer

valya

Impact: Low
Likelihood: Low

Root + Impact

Description

  • Some ERC20 tokens return false on failed transaction and do not revert. The return bool value of transfer() function is not checked and this may lead to wrong assumption that tokens are transferred when transfer() or transferFrom() returns false.

The issue is in the following code lines:

  1. Pot::closePot()

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);
​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

  1. Pot::_transferReward()

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

  1. ContestManager::fundContest():

function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
@> token.transferFrom(msg.sender, address(pot), totalRewards);
}

Risk

Likelihood:

  • The issue occurs when there is an attempt to transfer a token that returns false when a transfer failed.

Impact:

  • The protocol and user may compose a false assumption that transfer is successful, but it is not.

Proof of Concept

Some tokens do not return revert of transfer failure - https://github.com/d-xo/weird-erc20?tab=readme-ov-file#no-revert-on-failure

Recommended Mitigation

Use SafeERC20 from OpenZeppelin and the transaction will revert if the tranfer is not successful:

+ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
+ using SafeERC20 for IERC20;
​
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.safeTransfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
​
function _transferReward(address player, uint256 reward) internal {
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
}
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
- token.transferFrom(msg.sender, address(pot), totalRewards);
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!