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

Borrowers might lose their collateral assets before liquidation process itself

bhilare71

Summary

Borrowers might lose their collateral before liquidation, because of LiquidationPool::distributeAssets function's visibility being external, which can lead to premature distribution of assets to the holder as form of rewards and borrowers losing their collateral.

Vulnerability Details

In a vault, the collateral assets provided by the borrower are being distributed among the stake holders in the form of rewards , after the vault becomes under-collateralized, leading to liquidation of the vault, and then borrowers lose their collateral they deposited.

Which means the assets should only be distributed to holders, once liquidation process is being executed.

Liquidation process can be only executed liquidator by calling LiquidationPoolManager::runLiquidation() function as you can see below :

runLiquidation function
​
function runLiquidation(uint256 _tokenId) external
{
ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
manager.liquidateVault(_tokenId);
distributeFees();
ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](tokens.length);
uint256 ethBalance;
for (uint256 i = 0; i < tokens.length; i++)
{
ITokenManager.Token memory token = tokens[i];
if (token.addr == address(0))
{
ethBalance = address(this).balance;
if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
}
else
{
IERC20 ierc20 = IERC20(token.addr);
uint256 erc20balance = ierc20.balanceOf(address(this));
if (erc20balance > 0) {
assets[i] = ILiquidationPoolManager.Asset(token, erc20balance);
ierc20.approve(pool, erc20balance);
}
}
}
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
}

At the end we can see distributeAssets function being called in order to distribute the collateral assets provided by the borrower to all the holders

Below is the given distributeAssets function in detail:

distributeAssets function
​
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;
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);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}

Now we can see here that distributeAssets function is external , which means anyone can call this function, and while this function is called the rewards mapping are being updated for the holder with respect to the specific tokens present in the assets on the basis of their portion in the liquidation pool

​
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
​

And in the end , EUROs tokens are burned , so that no one can use them further.

if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);

AND ALSO THERE NO CHECKS OF AUTHORIZATION IN BETWEEN THE FUNCTION OR THE FUNCTIONS BEING CALLED.

The vulnerability lies in the visibility of this function i.e external, its arguments can be easily determined since everything is transparent on blockchain, and anyone can hence call this function, this can lead to premature unauthorized distribution of assets i.e rewards to the holders, since this was only meant to be during the liquidation event is being executed by the liquidator, but instead can be called before the event, and would lead to distribution of assets since the rewards mapping are getting updated in this function itself.

Impact

If distribution assets takes place before the liquidation event, then the borrower's won't be then able to take back their collateral and lose all their collateral in the smart vault and the borrowers further won't be able to borrow more even if their vault was not under-collateralized leading to a huge financial loss for them. This poses a risk for the borrowers for depositing their collateral.

Tools Used

Manual reviewing of codebase

Recommendations

To mitigate this vulnerability, the smart contract should implement access control to restrict the execution of the distributeAssets function to the authorized liquidator only, i.e using onlyLiquidator modifier, here like being used in runLiquidation function.

Just this small change would be enough for mitigating the potential risk.

Updates

Lead Judging Commences

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

distributeAssets-issue

Support

FAQs

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