The Standard

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

All Chainlink USD pair price feeds don't have 8 decimals

Summary

distributeAssets() calculates costInEuros by using:

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

where assetPriceUsd and priceEurUsd are raw values fetched through chainlink latestRoundData() function. This assumes that all price feeds will have the same decimal precision (apparently, 8 is the general supposition made by the protocol), but this is not the case. For example AMPL / USD feed has 18 decimals.

Any mismatch between the decimals of EUR/USD price feed and ASSET/USD price feed could cause incorrect calculation of -

breaking major functionality across the protocol like -

  • users getting lesser than expected rewards

  • users getting their EUROs burned unfairly

LiquidationPool.sol#L205-L241

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);
}

Vulnerability Details

Let us consider an example:

  • 1 EUR = 1.09621 USD. That is, Chainlink's priceEurUsd (8 decimal precision for this price feed)

  • Let's assume an asset TOX with 18 decimal places

  • 1 TOX = 1.09621 USD. That is, Chainlink's assetPriceUsd (18 decimal precision for this price feed)

  • Let _portion be equal to

  • Incorrect calculation as per L220:
    $$
    \begin{equation}
    \begin{split} costInEuros &= 10 * {10}^{18-18} * 1096210000000000000 \div 109621000 * 100000 \div 120000 \
    &= 10 * {10}^0 * 10000000000 * 100000 \div 120000 \
    &= 83333333333 \
    \end{split}
    \end{equation}
    $$

  • Correct expected calculation (modified for simplicity, see the recommendation section below for exact changes to be made):
    $$
    \begin{equation}
    \begin{split} costInEuros &= 10 * {10}^{18-18} * 109621000 \div 109621000 * 100000 \div 120000 \
    &= 10 * {10}^0 * 1 * 100000 \div 120000 \
    &= 8 \
    \end{split}
    \end{equation}
    $$

  • The incorrect value of costInEuros = 83333333333 will cause _portion to be evaluated to a much lower value in L223 since 83333333333 is in the denominator here. Therefore, the rewards calculated on L227 is extremely low for the user. Also, _position.EUROs will be set to 0 on L226.

Impact

  • users getting lesser than expected rewards

  • users getting their EUROs burned unfairly

  • incorrect calculations across protocol

Tools Used

Manual inspection

Recommendations

Consider calling decimals() function on the price feed contract to get the correct decimals and calculate the value based on the returned decimals:

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
+ uint256 DECIMAL_PRECISION_SAVER = 30;
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ uint256 decimalsEurUsd = uint256(Chainlink.AggregatorV3Interface(eurUsd).decimals());
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 decimalsAssetUsd = uint256(Chainlink.AggregatorV3Interface(asset.token.clAddr).decimals());
uint256 _portion = asset.amount * _positionStake / stakeTotal;
- uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
- * _hundredPC / _collateralRate;
+ uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * _hundredPC * uint256(assetPriceUsd) * (10 ** (DECIMAL_PRECISION_SAVER - decimalsAssetUsd)) / (uint256(priceEurUsd) * (10 ** (DECIMAL_PRECISION_SAVER - decimalsEurUsd)) * _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);
}
Updates

Lead Judging Commences

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

chainlink-decimals

informational/invalid

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

chainlink-decimals

informational/invalid

Support

FAQs

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