Tadle

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

Incompatibility with Fee-on-Transfer Tokens due to Strict Balance Comparison

Summary

A vulnerability in the TokenManager contract prevents the proper handling of fee-on-transfer tokens due to strict balance comparisons. This issue can cause the transfer to revert, breaking the protocol's claim of supporting all standard ERC20 tokens.

Vulnerability Details

  • Found in src/core/TokenManager.sol at Line 259

@>: strict balance comparison demands that the amount received on the _to side must equal the transferred amount from the _from side. However, this is not the case with fee-on-transfer tokens, where the amount received could be less due to an internal fee deduction.

233: function _transfer(
...
258:
259:@> if (toBalanceAft != toBalanceBef + _amount) {
260: revert TransferFailed();
...
262: }

Impact

The impact is significant as it breaks the protocol's ability to support all standard ERC20 tokens, especially fee-on-transfer tokens. This could prevent users from transferring certain tokens, leading to unexpected failures and disruptions.

Tools Used

Manual Review

Recommendations

Only account for changes in the fromBalance to accommodate fee-on-transfer tokens and ensure compatibility with all standard ERC20 tokens.

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.