Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ether_sky - The interest rate for the collateral asset is incorrectly calculated during liquidation #341

Closed
sherlock-admin3 opened this issue Sep 10, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Sep 10, 2024

ether_sky

Medium

The interest rate for the collateral asset is incorrectly calculated during liquidation

Summary

In the protocol, depositors can supply assets as collateral to the pool and earn interest from borrowers.
Borrowers, in turn, can borrow assets and are required to pay interest.
It's essential that interest rates are correctly calculated for both debt and collateral.
However, during liquidation, the interest rates for the collateral asset are incorrectly calculated.
Additionally, the underlyingBalance of the collateral asset is tracked inaccurately.

Vulnerability Detail

When liquidity changes (such as during deposits, repayments, borrowing, or withdrawals), the interest rate adjusts.

function updateInterestRates(
  DataTypes.ReserveData storage _reserve,
  DataTypes.ReserveSupplies storage totalSupplies,
  DataTypes.ReserveCache memory _cache,
  address _reserveAddress,
  uint256 _reserveFactor,
  uint256 _liquidityAdded,
  uint256 _liquidityTaken,
  bytes32 _position,
  bytes memory _data
) internal {
158:  vars.totalDebt = _cache.nextDebtShares.rayMul(_cache.nextBorrowIndex);
  
  (vars.nextLiquidityRate, vars.nextBorrowRate) = IReserveInterestRateStrategy(_reserve.interestRateStrategyAddress)
    .calculateInterestRates(
    _position,
    _data,
    DataTypes.CalculateInterestRatesParams({
165:      liquidityAdded: _liquidityAdded,
166:      liquidityTaken: _liquidityTaken,
      totalDebt: vars.totalDebt,
      reserveFactor: _reserveFactor,
      reserve: _reserveAddress
    })
  );

  _reserve.liquidityRate = vars.nextLiquidityRate.toUint128();
  _reserve.borrowRate = vars.nextBorrowRate.toUint128();

176if (_liquidityAdded > 0) totalSupplies.underlyingBalance += _liquidityAdded.toUint128();
177else if (_liquidityTaken > 0) totalSupplies.underlyingBalance -= _liquidityTaken.toUint128();
  );
}

Specifically, in line 158, totalDebt represents the current total debt of the asset reserve.
When liquidity increases (e.g., deposits or repayments), the _liquidityAdded value is positive (line 165).
Conversely, when liquidity decreases (e.g., borrowing or withdrawals), the _liquidityTaken value is positive (line 166).
These values are used to calculate new interest rates for both debt and collateral.
In lines 176 and 177, the underlyingBalance is updated based on changes in liquidity.

The liquidation process occurs in the executeLiquidationCall function.
In line 138, the _calculateAvailableCollateralToLiquidate function is called to determine how much collateral the liquidator and the protocol will receive.

function executeLiquidationCall(
  mapping(address => DataTypes.ReserveData) storage reservesData,
  mapping(uint256 => address) storage reservesList,
  mapping(address => mapping(bytes32 => DataTypes.PositionBalance)) storage balances,
  mapping(address => DataTypes.ReserveSupplies) storage totalSupplies,
  mapping(bytes32 => DataTypes.UserConfigurationMap) storage usersConfig,
  DataTypes.ExecuteLiquidationCallParams memory params
) external {
138:  (vars.actualCollateralToLiquidate, vars.actualDebtToLiquidate, vars.liquidationProtocolFeeAmount) =
  _calculateAvailableCollateralToLiquidate(
    collateralReserve,
    vars.debtReserveCache,
    vars.actualDebtToLiquidate,
    vars.userCollateralBalance,
    vars.liquidationBonus,
    IPool(params.pool).getAssetPrice(params.collateralAsset),
    IPool(params.pool).getAssetPrice(params.debtAsset),
    IPool(params.pool).factory().liquidationProtocolFeePercentage()
  );
}

The variable vars.actualCollateralToLiquidate represents the amount sent to the liquidator, while vars.liquidationProtocolFeeAmount is the protocol's fee.
The total amount removed from the collateral asset reserve should be the sum of these two values.

However, when updating the interest rates for the collateral asset, only vars.actualCollateralToLiquidate is considered in the _liquidityTaken value in line 222.

function _burnCollateralTokens(
  DataTypes.ReserveData storage collateralReserve,
  DataTypes.ExecuteLiquidationCallParams memory params,
  LiquidationCallLocalVars memory vars,
  DataTypes.PositionBalance storage balances,
  DataTypes.ReserveSupplies storage totalSupplies
) internal {
  DataTypes.ReserveCache memory collateralReserveCache = collateralReserve.cache(totalSupplies);
  collateralReserve.updateState(params.reserveFactor, collateralReserveCache);
  collateralReserve.updateInterestRates(
    totalSupplies,
    collateralReserveCache,
    params.collateralAsset,
    IPool(params.pool).getReserveFactor(),
    0,
222:    vars.actualCollateralToLiquidate,
    params.position,
    params.data.interestRateData
  );
}

As a result, vars.liquidationProtocolFeeAmount is excluded from the interest rate calculation.

Impact

Interest rates are a crucial component of the lending protocol.
Additionally, due to this issue, the underlyingBalance can no longer be trusted.

Code Snippet

https://github.com/sherlock-audit/2024-06-new-scope/blob/c8300e73f4d751796daad3dadbae4d11072b3d79/zerolend-one/contracts/core/pool/logic/ReserveLogic.sol#L158-L177
https://github.com/sherlock-audit/2024-06-new-scope/blob/c8300e73f4d751796daad3dadbae4d11072b3d79/zerolend-one/contracts/core/pool/logic/LiquidationLogic.sol#L138-L148
https://github.com/sherlock-audit/2024-06-new-scope/blob/c8300e73f4d751796daad3dadbae4d11072b3d79/zerolend-one/contracts/core/pool/logic/LiquidationLogic.sol#L222

Tool used

Manual Review

Recommendation

Ensure that vars.liquidationProtocolFeeAmount is also included in the interest rate calculation.

Duplicate of #401

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. labels Sep 20, 2024
@sherlock-admin3 sherlock-admin3 changed the title Smooth Carbon Narwhal - The interest rate for the collateral asset is incorrectly calculated during liquidation ether_sky - The interest rate for the collateral asset is incorrectly calculated during liquidation Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Oct 3, 2024
@WangSecurity WangSecurity added Medium A Medium severity issue. and removed High A High severity issue. labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants