The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: low
Invalid

runLiquidation() has no deadline & exposes liquidators to unlimited slippage which is dangerous when coupled with lack of access control

Summary

Unlike the implementation in many other protocols where the liquidator calls the liquidate() function to his own benefit, the current protocol allows anybody to call the runLiquidation() function in LiquidationPoolManager.sol which results in EUROs being deducted from the stakers' position and them being credited with the collateral in proportionate quantities.

Additonally, the value of the collateral going to be credited is calculated on-chain via the chainlink price feed inside distributeAssets() by calling Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData(), but none of the above functions allow specifying a deadline or a slippage parameter by way of a minimumOut variable.

These can result in the stakers/liquidators getting a much worse deal than they bargained for.

function runLiquidation(uint256 _tokenId) external {
ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
manager.liquidateVault(_tokenId);
....
....
}
@---> LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
}
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
@------------> (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
@------------> uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
....
....

Vulnerability details

Let's go through the following scenario:

  • Dorothy has deposited worth of ETH as collateral. The current chainlink price feed of ETH/EUR is say, which means she has deposited ETH. (This example is just for simplicity. Protocol actually uses ETH/USD & EUR/USD feeds to calculate the price of collateral in EUR).

  • Her max limit to borrow is worth of EUROs. This is because (€1000 + mintingFee) * collateralRate = .

  • She borrows by calling mint()

  • Price fluctuates & the chainlink price feed returns a value of 1.2 which means . This makes her eligible for liquidation.

  • Liam wishes to liquidate Dorothy. He calculates that if he liquidates now, he will get collateral worth at a discounted price.

  • There is another equivalent staker in the vault, Nash who is not interested in liquidating Dorothy if liquidation happens in a scenario where ETH/EUR dips below 1.001 within the next 5 minutes. He believes the price drop would be too fast in that case, or anticipates a black swan event thereon and is not confident of being able to quickly dispose of the acquired ETH at any profit. So he is fine with the current liquidation price point of 1.2, but certainly not below 1.001.

  • However, neither Liam nor Nash have any option of setting either the deadline or the slippage parameter while calling runLiquidation()

  • Worse still, any external user can call runLiquidation() to initiate liquidation, even a griefer.

  • So imagine that someone calls runLiquidation() which internally calls distributeAssets() on L80 which in turn calls chainlink latestRoundData() on L218 that is responsible to fetch the current chainlink price feed of the collateral/USD pair. Based on calculations in L220 the costInEuros is calculated, which is proportionately deducted from Liam's and Nash's EUROs balance and ETH is rewarded to them. Dorothy is now liquidated.

  • The above transaction can be stuck in the mempool (due to low transaction fee provided, network congestion or some other reason) for quite long like minutes, hours or sometimes even days. As such, even though the caller of runLiquidation() may have called it at a time when ETH/EUR was 1.2, it gets executed when the price feed is 1.0, breaching the limit Nash had in mind. Note that this price difference is not contingent upon the transaction being stuck in the mempool. This price dip may well happen within seconds due to market volatility, which is never considered a red flag due to lack of a slippage check like minimumOut.

  • Neither Nash nor Liam have any say now in whether they want to part with their EUROs or not. They are now stuck with the volatile, fast-declining ETH which thay are doubtful of being able to dispose of with any profits.

Impact

  • Unlimited slippage and risk of fund loss for the liquidators.

  • Can be an attack vector by a griefer too to result in the liquidators procuring a bad token at the cost of a good token.

Tools Used

Manual inspection

Recommendations

Multiple steps need to be taken:

  • Add a minimumCollateralValuation and deadline parameter inside the runLiquidation() function which the caller can choose.

  • Design a new function which allows each staker i.e. the liquidators to set their minimum threshold of collateral valuation in advance, like Nash wanted to in the above example. This could be specified in terms of a percentage dip from the current price or in absolute numbers. The percentage approach has the obvious benefit of allowing it to be set once and not tinkered with often for most market conditions, while the absolute number approach can be taken for specific niche market situations.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

t0x1c Submitter
over 1 year ago
hrishibhat Lead Judge
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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