A2-security - Incorrect Interest Rate Update Leads to Inflated Rates and Potential Protocol Insolvency

[sherlock-admin3]

A2-security

High

Incorrect Interest Rate Update Leads to Inflated Rates and Potential Protocol Insolvency

Summary

A vulnerability in the executeRepay function of BorrowLogic.sol causes interest rates to be calculated incorrectly due to updating rates before modifying the debt. This results in inflated interest rates and an overstated liquidity index. leading to insolvency as non-existent debt shares are accounted for, risking the ability of depositors to withdraw their funds even if all debts are settled.

Vulnerability Detail

The executeRepay function in BorrowLogic.sol contains a critical flaw in how it updates interest rates, leading to overestimation of repayment impact:

function executeRepay(...) external returns (DataTypes.SharesType memory payback) {
// ...
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);
// ...
}

• The issue arises from the sequence of operations:

  • updateInterestRates is called before modifying the cache.nextDebtShares.
  • repayDebt is called after updating interest rates, decreasing totalSupplies.debtShares.

This sequence results in the repaid amount being counted incorrectly in the interest rate calculation:

• The interest rates are updated based on the old debt amount, not reflecting the repayment The debt is then decreased after the interest rate update, leading to a mismatch.

• This incorrect ordering leads to an inflated borrowUsageRatio, resulting in higher interest rates than intended after repay operations.

• Please note that the existence of this bug can be confirmed by comparing the repay logic with the borrow logic in BorrowLogic.sol.

function executeBorrow(...) public returns (DataTypes.SharesType memory borrowed) {
// ...
(isFirstBorrowing, borrowed.shares) = b.borrowDebt(totalSupplies, params.amount, cache.nextBorrowIndex);
cache.nextDebtShares = totalSupplies.debtShares;

reserve.updateInterestRates(totalSupplies, cache, params.asset, IPool(params.pool).getReserveFactor(), 0, params.amount, params.position, params.data.interestRateData);
// ...
}

In the borrow function, cache.nextDebtShares is modified before updating interest rates, and liquidityTaken is considered. This inconsistency between borrow and repay operations further highlights the flaw in the repay logic, regardless of the specific interest module implementation since they call the same function.

• It's important to note that Zeroland is an Aave fork, and in Aave's implementation, debt shares are burned or minted before calling updateInterestRates(). This approach ensures that interest rate calculations are based on the most up-to-date debt state, avoiding the issues present in the current implementation.

Impact

The flaw in the executeRepay function leads to the overestimation of interest rates due to not reflecting the repayment in the debt amount (higher utilization ratio). This overestimation inflates the liquidity index with non-existent debt shares, causing insolvency. This means if all debts are repaid, there will not be enough liquidity to cover all depositors' withdrawals.

Code Snippet

• 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

Update the interest rates after modifying the debt in the executeRepay function:

function executeRepay(...) external returns (DataTypes.SharesType memory payback) {
// prev code ....

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

// same code ...
}

Duplicate of #413