The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Tokens with Fee-on-Transfer Mechanism Can Break the Protocol, Locking Funds Indefinitely

dyoff

[H-01] Tokens with Fee-on-Transfer Mechanism Can Break the Protocol, Locking Funds Indefinitely

Description

One of the tokens intended for collateral within the protocol, PAXG, utilizes a fee-on-transfer mechanism. However, this mechanism isn't compatible with the ERC20 logic in LiquidationPool. Within the LiquidationPool::distributeAssets() method, data is appended to the LiquidationPool::rewards mapping to track the holder's rewards portion. However, when this portion is transferred to the LiquidationPool on line 232, it leads to the pool balance being lower than the combined rewards portions stored in the mapping.

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
// ** code **
@> rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion; // @audit adding portion without the transfer fee
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
@> IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion); // @audit fee is deducted here
}
// ** code **
}

This discrepancy would cause a revert when the last holder attempts to claim their rewards using the LiquidationPool::claimRewards() method due to insufficient funds in the LiquidationPool. Notably, the earlier holders who have already claimed their rewards have implicitly compensated for their deducted transfer fees from the last holder's portion. This imbalance leads to the vulnerable aspect of this function, pinpointed specifically in line 175 of theLiquidationPool::claimRewards() function.

function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
require(_sent);
} else {
@> IERC20(_token.addr).transfer(msg.sender, _rewardAmount); // @audit this would revert with "insufficient balance"
}
}
​
}
}

Impact

This vulnerability poses a high risk as it affects PAXG, a proposed collateral asset within the protocol. Failure to support such tokens could lead to user funds being inaccessible. Additionally, the protocol intends to utilize other ERC20 tokens in the future, which could potentially encounter the same issue. For instance, USDT possesses a similar fee-on-transfer mechanism that is currently deactivated.

Proof of Concept

A code demonstration using Foundry to exhibit the inability of a holder to claim rewards can be accessed here.

Recommended Mitigation

To address such tokens, a suggested approach involves caching the LiquidationPool balance before executing a transferFrom to the contract. Subsequently, after the transfer, verifying the difference between the cached and current balances as the newly added balance.

Tools Used

Manual review

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

fee-on-transfer

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

fee-on-transfer

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.