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

`LiquidationPool.distributeAssets()` might leave non-zero allowance, leading to DoS of liquidations if tokens like USDT are used.

tricko

Summary

LiquidationPool.distributeAssets() might not fully transfer all tokens approved by LiquidationPoolManager, resulting in a non-zero allowance. If non-standard tokens, such as USDT, are used as collateral for the vaults, this could potentially lead to DoS of liquidations.

Vulnerability Details

During LiquidationPool.distributeAssets(), funds designated as rewards for stakers are moved from LiquidationPoolManager to the LiquidationPool contract. In order to do so, LiquidationPoolManager grants approval for its balances of all accepted ERC20 tokens to the LiquidationPool contract. However, in certain situations, not all of this allowance is utilized by LiquidationPool.distributeAssets(), resulting in non-zero allowances for specific tokens.

The code snippet below, extracted from the LiquidationPool.distributeAssets() function, illustrates the section where the token rewards' share (_portion) for each staker is calculated. If the _portion value in euros exceeds the staker's current EURO position, it is proportionally reduced.

uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}

As a consequence of this reduction, not all of the balance approved by LiquidationPoolManager is spent. While this might not pose an issue in most cases, certain non-standard ERC20 tokens, like USDT, only permit approving non-zero amounts if the current allowance is zero, reverting otherwise.

Therefore, if USDT or a similar token with such behavior is utilized as collateral, it could potentially result in a DoS situation for liquidations. This is because once one LiquidationPoolManager.runLiquidation() operation leaves an unspent allowance, subsequent calls to LiquidationPoolManager.runLiquidation() will fail when attempting to increase the allowance, leading to a revert.

Impact

All further liquidations will revert, unless the problematic token is removed from the accepted token list.

Tools Used

Manual Review.

Recommended Mitigation

Consider setting all the allowances to zero at the end of LiquidationPoolManager.runLiquidation(), as shown below.

diff --git a/LiquidationPoolManager.sol b/LiquidationPoolManager.mod.sol
index 3fb0203..01a1442 100644
--- a/LiquidationPoolManager.sol
+++ b/LiquidationPoolManager.mod.sol
@@ -79,6 +79,11 @@ contract LiquidationPoolManager is Ownable {
}
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
+ for (uint256 i = 0; i < tokens.length; i++) {
+ ITokenManager.Token memory token = tokens[i];
+ IERC20 ierc20 = IERC20(token.addr);
+ ierc20.approve(pool, 0);
+ }
}
Updates

Lead Judging Commences

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

distributeAssets-issue

tricko Submitter
over 1 year ago
hrishibhat Lead Judge
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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