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

Potential Failure to Handle Fee-on-Transfer Tokens Correctly

salem

Summary

The TokenManager contract does not properly account for fee-on-transfer tokens, which could lead to transaction failures and accounting discrepancies if such tokens are used.

Vulnerability Details

The contract assumes that when transferring tokens, the full amount sent will be received by the recipient. This assumption is evident in the _transfer function, which checks for exact balance changes:

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L252

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

For fee-on-transfer tokens, these checks would fail because the recipient receives less than the sent amount due to transfer fees.

Impact

  • Transaction Failures: If a fee-on-transfer token is whitelisted and used, functions like tillIn and withdraw could fail due to the strict balance checks in _transfer.

  • Accounting Discrepancies: The contract doesn't account for the possibility of receiving fewer tokens than expected, which could lead to incorrect balance tracking.

  • Potential Fund Lock: In severe cases, this could lead to funds being locked and DoS in the contract if transfers consistently fail.

Tools Used

Manual Review

Recommendations

Modify balance checks for discrepancy

if (fromBalanceAft > fromBalanceBef - _amount || fromBalanceAft < fromBalanceBef - _amount - maxFee) {
revert TransferFailed();
}
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.