Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

The protocol does not take into account weird ERC20 tokens, such as Fee on Transfer tokens.

amaron

Summary

TokenManager::_transferdoes not take into account that some tokens have, or may have in the future (usdc) fee on transfer. consequently, the transfer of the tokens might revert.

Vulnerability Details

the _transfer function, validate that the tokens have been transferred with the following logic:

function _transfer(address _token, address _from, address _to, uint256 _amount, address _capitalPoolAddr)
internal
{
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
​
if (_from == _capitalPoolAddr && IERC20(_token).allowance(_from, address(this)) == 0x0) {
ICapitalPool(_capitalPoolAddr).approve(_token);
}
​
_safe_transfer_from(_token, _from, _to, _amount);
​
uint256 fromBalanceAft = IERC20(_token).balanceOf(_from);
uint256 toBalanceAft = IERC20(_token).balanceOf(_to);
​
@> if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed(); //@audit - low - fee on transfer.
}
​
@> if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}
}

however, some weird ERC20s, such as Fee on Transfer tokens, might not pass these checks, and the whole transfer will revert.

Impact

Fee on Transfer tokens (or potentially if USDC will add Fee on Transfer mechanism) will not be able to pass the _transfer function validation, therefore it will revert and cause unexpected errors, and potentially loss of funds.

Tools Used

manual review

Recommendations

consider implementing different validation, or use SafeERC20.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-FOT-Rebasing

Valid medium, there are disruptions to the ability to take market actions. The following functions will be disrupted without the possibiliy of reaching settlement, since the respective offers cannot be created/listed regardless of mode when transferring collateral token required to the CapitalPool contract or when refunding token from user to capital pool during relisting. So withdrawal is not an issue - `createOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96-L102) - `listOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L355-L362) - `relistOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515-L521) - `createTaker()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831-L836) I believe medium severity is appropriate although the likelihood is high and impact is medium (only some level of disruption i.e. FOT tokens not supported and no funds at risk)

Support

FAQs

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