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

Protocol is not compatible with fee-on-transfer ERC20 tokens

rbserver

Summary

Although https://github.com/Cyfrin/2024-08-tadle?tab=readme-ov-file#compatibilities states that this protocol should be compatible with ERC20 (any token that follows the ERC20 standard), this protocol actually cannot support the fee-on-transfer ERC20 tokens that follow all specifications of https://eips.ethereum.org/EIPS/eip-20.

Vulnerability Details

When calling the following tillIn function, _transfer(_tokenAddress, _accountAddress, capitalPoolAddr, _amount, capitalPoolAddr) would be executed when the _tokenAddress input is not wrappedNativeToken.

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/TokenManager.sol#L56-L103

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) {
...
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
);
}
...
}

Then, in the following _transfer function, _safe_transfer_from(_token, _from, _to, _amount) is executed. When the corresponding token is a fee-on-transfer ERC20 token, such _safe_transfer_from function call would transfer a token amount that equals _amount minus the associated transfer fee to _to. Since the actual token amount received by _to is less than _amount, toBalanceAft != toBalanceBef + _amount would be true, and such _transfer and tillIn function calls would revert.

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/TokenManager.sol#L233-L262

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();
}
}

Impact

This protocol cannot support the fee-on-transfer ERC20 tokens even though such tokens can follow all specifications of https://eips.ethereum.org/EIPS/eip-20. This breaks the requirement that this protocol should be compatible with ERC20 (any token that follows the ERC20 standard), which is stated in https://github.com/Cyfrin/2024-08-tadle?tab=readme-ov-file#compatibilities.

Tools Used

Manual Review

Recommended Mitigation

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/TokenManager.sol#L259-L261 can be updated to the following code.

if (toBalanceAft <= toBalanceBef || 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.