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

Inability of TokenManager.sol's _transfer Function to Handle Fee-on-Transfer Tokens Like PAXG and STA

eta

Summary

The _transfer function in TokenManager.sol fails with fee-on-transfer tokens like PAXG and STA because the actual balances after transfer don't match the expected values, causing errors. This flaw also breaks the tillIn function, which depend on _transfer.

Vulnerability Details

The _transfer function in the TokenManager.sol contract is unable to handle fee-on-transfer tokens such as PAXG and STA. These tokens deduct a fee during transfers, which causes the expected balances to differ from the actual balances. Specifically, the conditions fromBalanceAft != fromBalanceBef - _amount and toBalanceAft != toBalanceBef + _amount are not met, resulting in an error that prevents the function from operating correctly.
src/core/TokenManager.sol:_transfer_L255-L261

​
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) {
revert TransferFailed();
}
}

Since the tillIn function within TokenManager.sol rely on _transfer, this issue causes such functions to fail as well.
src/core/TokenManager.sol:tillIn_L93-L99

​
function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
/// @notice return if amount is 0
if (_amount == 0) {
return;
}
​
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (capitalPoolAddr == address(0x0)) {
revert Errors.ContractIsNotDeployed();
}
​
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
/// @notice token is ERC20 token
@=> _transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
​
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
​

Impact

The _transfer function's inability to process fee-on-transfer tokens leads to a failure in executing related functions such as tillIn, making these operations impossible to perform within the contract.

Tools Used

Manual Review

Recommendations

It is recommended to check the balance of Token before and after the transfer to see how much tokens were received to handles fee on transfer tokens.

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!