Tadle

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

Transfer Fee tokens will revert transfers, causing DoS for users

Summary

The tillIn() function transfers tokens to a capital pool. However, the internal _transfer() function does not account for tokens that charge a fee on transfer. This leads to an erroneous balance check, causing the transaction to revert even when the transfer is successful but includes a fee.

Vulnerability Details

The tillIn() function handles token transfers to a capital pool, distinguishing between native tokens and ERC20 tokens. When handling ERC20 tokens, it calls the internal _transfer() function:

function tillIn(...)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
...
if (_tokenAddress == wrappedNativeToken) {
if (msg.value <= _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
@> _transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
ICapitalPool(_capitalPoolAddr).approve(address(this));
}
_safe_transfer_from(_token, _from, _to, _amount);
uint256 fromBalanceAft = IERC20(_token).balanceOf(_from);
uint256 toBalanceAft = IERC20(_token).balanceOf(_to);
if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
>@ if (toBalanceAft != toBalanceBef + _amount) { //@audit will
revert TransferFailed();
}
}

The vulnerability lies in the _transfer function's balance checks. It assumes that the amount transferred will be exactly equal to the amount deducted from the sender and added to the recipient. This assumption fails for tokens that charge a transfer fee (e.g., STA, PAXG), as the balance added to the recipient will be less than the amount deducted from the sender.

Affected LoC:

Impact

For tokens that charge a transfer fee, the balance check will fail, causing the transaction to revert. This results in a denial of service (DoS) for users attempting to transfer such tokens, as the contract does not accommodate the fee-on-transfer mechanism.

Tools Used

Manual

Recommendations

To address this issue, the balance checks should be modified to accommodate tokens that charge a transfer fee. Instead of directly comparing the expected balance changes, the function should verify that the recipient received at least the minimum expected amount.

function _transfer(...) internal {
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
if (
_from == _capitalPoolAddr &&
IERC20(_token).allowance(_from, address(this)) == 0x0
) {
ICapitalPool(_capitalPoolAddr).approve(address(this));
}
_safe_transfer_from(_token, _from, _to, _amount);
uint256 fromBalanceAft = IERC20(_token).balanceOf(_from);
uint256 toBalanceAft = IERC20(_token).balanceOf(_to);
if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
- if (toBalanceAft != toBalanceBef + _amount) {
+ if (toBalanceAft < toBalanceBef + _amount) {
revert TransferFailed();
}
}
Updates

Lead Judging Commences

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