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

TokenManger.sol doesn't work with fee on transfer tokens

pandasec

Summary

TokenManger.sol doesn't work with fee on transfer tokens and will revert if such token is used, making the use of such tokens impossible. Tokens that are supposed to work as collateral(for example USDC or USDT) can also add a fee on transfer in the future and this will make withdrawing from the contract impossible.

Vulnerability Details

The _transfer function in TokenManager.sol doesn't account for fee-on-transfer tokens and thus will revert in such cases, which makes it impossible for using fee-on-transfer tokens.

if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
​
if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}

The checks will always fail if a fee-on-transfer token is used, either on deposit(tillIn function) or withdraw. An even worse-case scenario is a token enabling fee later on, after deposits have already been made so it'll make withdrawing of the tokens impossible.

Impact

Inability to use fee-on-transfer tokens and if a token adds a fee later on, that'll make already deposited tokens in the contract unable to be withdrawn and funds will be stuck.

Tools Used

Manual review

Recommendations

To support such tokens, do not revert in _transfer and change the tillIn function to check the balances after and emit the TillIn event with correct amount.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.