Tadle

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

Transfers don't support fee-on-transfer

Summary

The current implementation does not support pre-markets that use fee-on-transfer tokens. It prevents tokens from having transfer fees, which completely contradicts the goal of supporting all tokens that adhere to the ERC20 standard.

Vulnerability Details

Most functions that interact with the marketplace, such as creating an offer or a taker, call the function tillIn in the TokenManager.sol contract. This function, in turn, calls the _transfer function, which performs the following check:

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

This code reverts if any change in the amount occurs during the transfer, which prevents the use of fee-on-transfer tokens.

Impact

Any marketplace that deals with tokens having transfer fees becomes obsolete because most functions interacting with the marketplace will revert.

Tools Used

Manual review.

Recommendations

To handle fee-on-transfer tokens, you should check how many tokens the _to address received, calculate the difference between the amount and use the current amount received by the user to update the storage. Implementing a function similar to Compoud's doTransferIn could be a solution.

Updates

Lead Judging Commences

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

Give us feedback!