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 is incorrectly updated when debt is repaid #358

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

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 10, 2024

ether_sky

Medium

The interest rate is incorrectly updated when debt is repaid

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 repayment, the interest rate for debt assets is updated incorrectly.

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

  if (_liquidityAdded > 0) totalSupplies.underlyingBalance += _liquidityAdded.toUint128();
  else 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 the executeRepay function, the interest rates are updated in line 139.
At this point, cache.nextDebtShares does not account for the repaid debt amount—it still equals the current debt shares.

function executeRepay(
  DataTypes.ReserveData storage reserve,
  DataTypes.PositionBalance storage balances,
  DataTypes.ReserveSupplies storage totalSupplies,
  DataTypes.UserConfigurationMap storage userConfig,
  DataTypes.ExecuteRepayParams memory params
) external returns (DataTypes.SharesType memory payback) {
139:  reserve.updateInterestRates(
    totalSupplies,
    cache,
    params.asset,
    IPool(params.pool).getReserveFactor(),
    payback.assets,
    0,
    params.position,
    params.data.interestRateData
  );

  payback.shares = balances.repayDebt(totalSupplies, payback.assets, cache.nextBorrowIndex);
152:  cache.nextDebtShares = totalSupplies.debtShares;
}

Only later, in line 152, do we update cache.nextDebtShares by subtracting the repaid amount from the current debt shares.
However, this updated nextDebtShares should be used when calculating the new interest rate.

Impact

Interest rates are a critical part of the lending protocol, and repayment can happen frequently.
Each time a repayment occurs, the interest rates for debt assets are being incorrectly recalculated.
The total debt has a significant impact on the calculation of interest rates.

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/BorrowLogic.sol#L139-L152

Tool used

Manual Review

Recommendation

function executeRepay(
  DataTypes.ReserveData storage reserve,
  DataTypes.PositionBalance storage balances,
  DataTypes.ReserveSupplies storage totalSupplies,
  DataTypes.UserConfigurationMap storage userConfig,
  DataTypes.ExecuteRepayParams memory params
) external returns (DataTypes.SharesType memory payback) {
+  payback.shares = balances.repayDebt(totalSupplies, payback.assets, cache.nextBorrowIndex);
+  cache.nextDebtShares = totalSupplies.debtShares;

  reserve.updateInterestRates(
    totalSupplies,
    cache,
    params.asset,
    IPool(params.pool).getReserveFactor(),
    payback.assets,
    0,
    params.position,
    params.data.interestRateData
  );

-  payback.shares = balances.repayDebt(totalSupplies, payback.assets, cache.nextBorrowIndex);
-  cache.nextDebtShares = totalSupplies.debtShares;
}

Duplicate of #413

@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 is incorrectly updated when debt is repaid ether_sky - The interest rate is incorrectly updated when debt is repaid Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Oct 3, 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 High A High severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants