Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

The protocol is incompatible with fee on transfer tokens

bube

Links

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L259-L261

Summary

The purpose of the TokenManager::_transfer function is to transfer ERC20 tokens between addresses. However, it is incompatible with fee-on-transfer tokens due to strict balance checks before and after the transfer.

Vulnerability Details

The TokenManager::_transfer function is used to transfer a given token from _from address to _to address:

​
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(address(this));
}
​
_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();
}
​
@> if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}
}
​

The function checks the balance before and after the transfer of the sender and the receiver. Also, according to the docs of the contest the protocol should be compatible with every ERC-20 token:

Tokens:
- ETH
- WETH
- ERC20 (any token that follows the ERC20 standard)
​

But some ERC-20 tokens are fee on transfer tokens (STA, PAXG, USDC can be in the future) and the function will not work with them because of the check toBalanceAft != toBalanceBef + _amount that will always revert.

Impact

The balance checks before and after the transfer assume that the exact _amount of tokens will be transferred. This assumption fails for fee-on-transfer tokens, which deduct a fee during the transfer. As a result the receiver's balance will increase by less than _amount and the _transfer function will always revert. That means every function that relies on the _transfer function will also revert. The _transfer function is called in TokenManager::tillIn function that is critical for the protocol functionality because it transfers a token from the user to the capital pool and the tillIn function is used in many important for the protocol functions.
If fee on transfer token is used the functionality of the protocol will be broken.

Tools Used

Manual Review

Recommendations

Implement logic to account for the fee deducted during the transfer. This involves estimating the fee and adjusting the balance checks accordingly.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 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.