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

Incorrect price calculation in the asset distribution

zigtur

Summary

The price calculation in the asset distribution mechanism does not take the Chainlink
Oracle decimals into calculation.

distributeAssets assumes and inexplicitly requires the asset's USD feed's decimals to be the same
than the EUR/USD feed's decimals (decimals = 8).

Vulnerability Details

The current implementation of the distributeAssets function of LiquidationPool assumes that the
ASSET/USD price decimals is the same than the EUR/USD price decimals.
The current EUR/USD price decimals is 8.

However, there exist tokens with USD price feed's decimals != 8.
When the tokens with USD price feed's decimals != 8, the token price calculation in distributeAssets
will be incorrect.

This vulnerability doesn't seem to be exploitable for now.
But, if new tokens are supported in the support, it might be the case.
That is why severity is medium and not high.

Impact

A distributed asset can be under priced or over priced due to incorrect calculations during distribution.

In case of over pricing, this could lead to the burning of stakers' EUROs position for a small amount of input asset.

Tools Used

Scope:

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L220

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L218

In the following calculations, costInEuros could be incorrect if assertPriceUsd and priceEurUsd
are not using the same decimals.

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;

Recommendations

In distributeAssets, consider adding an explicit check to make sure that ASSET/USD feed's
decimals == EUR/USD feed's decimals. The following diff shows such a fix.

diff --git a/contracts/LiquidationPool.sol b/contracts/LiquidationPool.sol
index 9b8e593..ea8338a 100644
--- a/contracts/LiquidationPool.sol
+++ b/contracts/LiquidationPool.sol
@@ -205,6 +205,7 @@ contract LiquidationPool is ILiquidationPool {
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ uint256 EurUsdDecimals = Chainlink.AggregatorV3Interface(eurUsd).decimals();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
@@ -215,6 +216,7 @@ contract LiquidationPool is ILiquidationPool {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
+ require(EurUsdDecimals == Chainlink.AggregatorV3Interface(asset.token.clAddr).decimals(), "unexpected decimals");
(,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)

Important Note

:warning: This vulnerability can also impact PriceCalculator.sol which is out of the audit scope.

Updates

Lead Judging Commences

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

precision

Support

FAQs

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