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

Some ERC-20 tokens will revert the transaction due to wrong validation

demorextess

Summary

According to the system documents, any token which follows ERC-20 standards should be supported by the system.

Compatibilities:
Blockchains:
- Ethereum/Any EVM
Tokens:
- ETH
- WETH
- ERC20 (any token that follows the ERC20 standard)

But some ERC-20 tokens are not supported in the system due to problematic if statement.

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

Vulnerability Details

ERC-20 tokens may have many additional features or behaviours ( see: https://github.com/d-xo/weird-erc20 ). Following if statements in _transfer() is problematic for those tokens:

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

We can give fee-on-transfer tokens as an example in this situation.

PoC

Let say a fee-on-transfer token added the system for peer-to-peer exchange and Alice wants to exchange her fee-on-transfer token.
In this situation, she can't send her tokens to system because it will be reverted in following line:

if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}

Because of fee, toBalanceAft will not be equal to before + amount

Impact

Missing main functionality

Tools Used

Manual Review

Recommendations

Instead of checking the equality, updating the amount parameter ( also collateral ) based on the received amounts of tokens will solve the problem

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.