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

Incompatibility with fee-on-transfer and rebasing ERC20 tokens leads to fund loss

0xnbvc

Vulnerability Details

The PreMarkets and TokenManager contracts are intended to be compatible with "any token that follows the ERC20 standard," according to the documentation.

However, some tokens that adhere to the ERC20 standard have additional features that can lead to fund loss, such as fee-on-transfer tokens and rebasing tokens (e.g. STA, PAXG etc).

Here are the different call flows for managing funds in the protocol:

- TokenManager::tillIn() -> _transfer() -> _safe_transfer_from() -> Token::transferFrom()
- TokenManager::withdraw() -> _transfer() -> _safe_transfer_from() -> Token::transferFrom()
- TokenManager::withdraw() -> _safe_transfer_from() -> Token::transferFrom()

The _transfer() function is incompatible with fee-on-transfer and rebasing tokens and will always revert:

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

The _safe_transfer_from() function is a standard transferFrom() function and will lead to a loss of funds when users try to withdraw their ERC20 tokens:

TokenManager.sol#L170-L181

} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
>> _safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}

Impact

Incompatibility with standard ERC20 tokens can lead to a loss of funds for users and disrupt the protocol's functionality when the admin attempts to add ERC20 standard fee-on-transfer or rebasing tokens to the whitelist.

Recommendations

Do not allow fee-on-transfer or rebasing tokens in the whitelist, and therefore, do not claim compatibility with "any token that follows the ERC20 standard".

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.