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

TokenManager Does Not Support Fee-On-Transfer Tokens

praise03

Summary

In the contest details, the platform claims to support all ERC20 token implementations, but fails to account for fee on transfer tokens

Vulnerability Details

Whenever a token that takes a fee on transfer is to be transfered to or from the capitalPool, it will always revert with a ***TransferFailed() *** error, due to the before and after balance checks implemented in the TokenManager's _transfer() function.

uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
​
//...TransferLogic
​
if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
//will always revert for fee on transfer tokens
if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}

POC

//Fee on Transfer token implementation
function transferFrom(address from, address to, uint256 value) public override returns (bool) {
address spender = _msgSender();
_spendAllowance(from, spender, value);
uint256 _fee = value * 5 / 100;
uint256 transfervalue = value - _fee;
​
_transfer(from, address(this), _fee);
_transfer(from, to, transfervalue);
return true;
}
​
//will always revert with TransferFailed() error
function testRevertForFeeTokens() public {
vm.expectRevert();
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(feeToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
}

Tools Used

Manual Review

Recommendations

In the tokenManager's transfer function, consider implementing the amount deposited/withdrawn as the difference between the previous balance and after balance. Rather than reverting if the difference != amount

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.