Honour - GenericLogic::calculateUserAccountData assumes same price decimals for all price feeds

[sherlock-admin3]

Honour

Medium

GenericLogic::calculateUserAccountData assumes same price decimals for all price feeds

Summary

Pool assumes that all oracles have the same price decimals, while it's a rule of thumb that USD feeds use 8 decimals and ETH feeds use 18 decimals, this is not always the case as we can see with AML/USD for example with 18 decimals, this can lead to severe miscalculation of positions health factor

Root Cause

In GenericLogic::calculateUserAccountData

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/GenericLogic.sol#L106-L119

  function calculateUserAccountData(
    mapping(address => mapping(bytes32 => DataTypes.PositionBalance)) storage _balances,
    mapping(address => DataTypes.ReserveData) storage reservesData,
    mapping(uint256 => address) storage reservesList,
    DataTypes.CalculateUserAccountDataParams memory params
  ) internal view returns (uint256, uint256, uint256, uint256, uint256, bool) {
    if (params.userConfig.isEmpty()) {
      return (0, 0, 0, 0, type(uint256).max, false);
    }

    //...

@>    vars.assetPrice = IPool(params.pool).getAssetPrice(vars.currentReserveAddress);

      if (vars.liquidationThreshold != 0 && params.userConfig.isUsingAsCollateral(vars.i)) {
@>      vars.PositionBalanceInBaseCurrency = _getPositionBalanceInBaseCurrency(
          _balances[vars.currentReserveAddress][params.position], currentReserve, vars.assetPrice, vars.assetUnit
        );

@>      vars.totalCollateralInBaseCurrency += vars.PositionBalanceInBaseCurrency;

        if (vars.ltv != 0) {
          vars.avgLtv += vars.PositionBalanceInBaseCurrency * (vars.ltv);
        } else {
          vars.hasZeroLtvCollateral = true;
        }

        vars.avgLiquidationThreshold += vars.PositionBalanceInBaseCurrency * vars.liquidationThreshold;
      }

      if (params.userConfig.isBorrowing(vars.i)) {
        vars.totalDebtInBaseCurrency += _getUserDebtInBaseCurrency(
          _balances[vars.currentReserveAddress][params.position], currentReserve, vars.assetPrice, vars.assetUnit
        );
      }

      unchecked {
        ++vars.i;
      }
    }

    unchecked {
      vars.avgLtv = vars.totalCollateralInBaseCurrency != 0 ? vars.avgLtv / vars.totalCollateralInBaseCurrency : 0;
      vars.avgLiquidationThreshold =
        vars.totalCollateralInBaseCurrency != 0 ? vars.avgLiquidationThreshold / vars.totalCollateralInBaseCurrency : 0;
    }

    vars.healthFactor = (vars.totalDebtInBaseCurrency == 0)
      ? type(uint256).max
      : (vars.totalCollateralInBaseCurrency.percentMul(vars.avgLiquidationThreshold)).wadDiv(vars.totalDebtInBaseCurrency);
    return (
      vars.totalCollateralInBaseCurrency,
      vars.totalDebtInBaseCurrency,
      vars.avgLtv,
      vars.avgLiquidationThreshold,
      vars.healthFactor,
      vars.hasZeroLtvCollateral
    );
  }

The _getPositionBalanceInBaseCurrency function returns the position balance in base currency with the price feed precision.

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/pool/logic/GenericLogic.sol#L207-L219

This would lead to miscalculations of positions health if price feeds have different decimals.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Significant miscalculations in positions health

PoC

No response

Mitigation

1. A strict mitigation would be to verify oracle decimals on setReserveConfiguration

  function setReserveConfiguration(
    mapping(address => DataTypes.ReserveData) storage _reserves,
    address asset,
    address rateStrategyAddress,
    address source,
    DataTypes.ReserveConfigurationMap memory config
  ) public {
    require(asset != address(0), PoolErrorsLib.ZERO_ADDRESS_NOT_VALID);
    _reserves[asset].configuration = config;

    // set if values are non-0
    if (rateStrategyAddress != address(0)) _reserves[asset].interestRateStrategyAddress = rateStrategyAddress;
    if (source != address(0)) _reserves[asset].oracle = source;

    require(config.getDecimals() >= 6, 'not enough decimals');
+   require(IAggregatorV3Interface(source).decimals()  == 8, 'invalid oracle decimals');//or 18 depending on base currency

    // validation of the parameters: the LTV can
    // only be lower or equal than the liquidation threshold
    // (otherwise a loan against the asset would cause instantaneous liquidation)
    require(config.getLtv() <= config.getLiquidationThreshold(), PoolErrorsLib.INVALID_RESERVE_PARAMS);

    if (config.getLiquidationThreshold() != 0) {
      // liquidation bonus must be bigger than 100.00%, otherwise the liquidator would receive less
      // collateral than needed to cover the debt
      require(config.getLiquidationBonus() > PercentageMath.PERCENTAGE_FACTOR, PoolErrorsLib.INVALID_RESERVE_PARAMS);

      // if threshold * bonus is less than PERCENTAGE_FACTOR, it's guaranteed that at the moment
      // a loan is taken there is enough collateral available to cover the liquidation bonus
      require(
        config.getLiquidationThreshold().percentMul(config.getLiquidationBonus()) <= PercentageMath.PERCENTAGE_FACTOR,
        PoolErrorsLib.INVALID_RESERVE_PARAMS
      );

      emit PoolEventsLib.CollateralConfigurationChanged(
        asset, config.getLtv(), config.getLiquidationThreshold(), config.getLiquidationThreshold()
      );
    }
  }

2. A more lenient mitigation would be to normalize all price feed to the same decimals in _getPositionBalanceInBaseCurrency

  function _getPositionBalanceInBaseCurrency(
    DataTypes.PositionBalance storage _balance,
    DataTypes.ReserveData storage reserve,
    uint256 assetPrice,
    uint256 assetUnit
  ) private view returns (uint256) {
    uint256 normalizedIncome = reserve.getNormalizedIncome();//this does liner interest calculation to get normalizedIncome (liquidityIndex)
-   uint256 balance = (_balance.supplyShares.rayMul(normalizedIncome)) * assetPrice;
+   uint256 balance = (_balance.supplyShares.rayMul(normalizedIncome)) * assetPrice * 1e18;

    unchecked {
-     return balance / assetUnit ;
+     return balance / (assetUnit * IAggregatorV3Interface(reserve.oracle).decimals());//all balance price in 1e18
    }
  }

Duplicate of #166