Anyone can manipulate the AL ratio.

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x645ef5fe0b6086692dd399b8a675ea85fef135ed5a8329bac13054d8b46fdab0 Severity: medium

Description: Description\ For the LovDSR vault, maintaining the A/L ratio within the target range is crucial to maintain the desired exposure. Various operations are tied to this ratio, and any action that pushes it outside the defined range is reversed. Thus, ensuring the ratio remains within the valid range is most important. However, there is a risk of manipulation by any user, potentially causing the protocol to freeze. While such malicious activity could result in the loss of funds for the malicious users, there may be scenarios where they can recover most of their funds.

Attack Scenario\ Imagine that the LovDSR vault currently holds 3000 sDAI, with 300 sDAI contributed by users and 2700 sDAI obtained through borrowing USDC. The 300 sDAI includes a deposit of 200 sDAI from User A.

User A repays 900 USDC to the LovDSR vault and this is possible due to the absence of access checking.

function repay(uint256 amount, address borrower) external override returns (uint256 amountRepaid) {
@1:     uint256 _debtBalance = debtToken.balanceOf(borrower);     // 18 dp
@2:     uint256 _maxRepayAmount = _debtBalance.scaleDown(_assetScalar, OrigamiMath.Rounding.ROUND_UP);   // asset's dp

    uint256 _debtToTransfer;  // 18 dp
    if (amount < _maxRepayAmount) {
@3:       amountRepaid = amount;
@4:        _debtToTransfer = amount.scaleUp(_assetScalar);
    } else {
        amountRepaid = _maxRepayAmount;
        _debtToTransfer = _debtBalance;
    }

    if (amountRepaid != 0) {
@5:     _repay(msg.sender, borrower, _borrowerConfig, amountRepaid, _debtToTransfer);
    }
}

@1: debtBalance = 2700 * 1e18 @2: _maxRepayAmount = 2700 * 1e6 @3: amountRepaid = 900 * 1e6 @4: _debtToTransfer = 900 * 1e18 @5: _repay function is invoked.

function _repay(address from,  address borrower,  BorrowerConfig storage borrowerConfig, uint256 repayAmount, uint256 debtToTransfer) private {
@6:    debtToken.safeTransferFrom(borrower, address(idleStrategyManager), debtToTransfer);
}

@6: we burn the debt 900 USDC from LovDSR vault.

Another borrower borrows this amount, or oUSDC investors exit their investment, thereby preventing the LovDSR vault from borrowing USDC in the future.

The A/L ratio shifts from 10/9 to 10/6 (= 3000 / 1800),

Now any investment in the LovDSR will be reverted.

function investWithToken(
    address account,
    IOrigamiInvestment.InvestQuoteData calldata quoteData
) external virtual override onlyLovToken returns (
    uint256 investmentAmount
) {
    if (cache.liabilities != 0) {
        cache.assets = reservesBalance();
        cache.liabilities = liabilities(IOrigamiOracle.PriceType.SPOT_PRICE);
        uint128 newAL = _assetToLiabilityRatio(cache);
@7:        _validateALRatio(userALRange, oldAL, newAL, AlValidationMode.HIGHER_THAN_BEFORE);
    }
}

@7: revert here

The same applies to exiting investments

function exitToToken(
    address /*account*/,
    IOrigamiInvestment.ExitQuoteData calldata quoteData,
    address recipient
) external virtual override onlyLovToken returns (
    uint256 toTokenAmount,
    uint256 toBurnAmount
) {
    if (cache.liabilities != 0) {
        cache.assets = reservesBalance();
    cache.liabilities = liabilities(IOrigamiOracle.PriceType.SPOT_PRICE);
        uint128 newAL = _assetToLiabilityRatio(cache);
@8:        _validateALRatio(userALRange, oldAL, newAL, AlValidationMode.LOWER_THAN_BEFORE);
    }
}

@8: revert here

The rebalanceDown function cannot help because the LovDSR vault is unable to borrow USDC because there is no available USDC.

To interact with the LovDSR, one potential solution is to repay the outstanding debt. This could involve using new USDC or utilizing reserves from the LovDSR vault. With the LovDSR still holding 3000 sDAI, converting 1800 sDAI to 1800 USDC for repayment would leave 1200 sDAI remaining. In this scenario, User A could retain 800 sDAI (1200 * 2 / 3), resulting in a loss of 300 sDAI.

Of course, this example serves to illustrate one potential solution. In reality, encountering such a malicious user might be unlikely, but it's important to consider and mitigate any potential risks.

Attachments

1. **Proof of Concept (PoC) File**

2. Revised Code File (Optional)

   Restrict repayment of debt to borrowers only. Or restrict repayment to elevated users only.

[FCSE507]

Another potential scenario:

The LovDSR vault currently holds 3000 sDAI, with 300 sDAI contributed by users and 2700 sDAI obtained through borrowing USDC. At this point, its debt ceiling is set at 3000.

An elevated user updates the debt ceiling to 2500 USDC.

function setBorrowerDebtCeiling(address borrower, uint256 newDebtCeiling) external override onlyElevatedAccess {
      BorrowerConfig storage borrowerConfig = _getBorrowerConfig(borrower);
      emit DebtCeilingUpdated(borrower, borrowerConfig.debtCeiling, newDebtCeiling);
      borrowerConfig.debtCeiling = newDebtCeiling;

      // The debt token balances for both the idle strategy and borrower are checkpoint
      // to make the Global utilisation rate more accurate
      _checkpointDebtTokenBalances(borrower);
@1:      _refreshBorrowersInterestRate(borrower, borrowerConfig, false);
}

@1: The validateUR parameter is false, so it will not be reverted.

A malicious user repays 200 USDC, disrupting the A/L ratio and pushing it outside the target range. To rectify the ratio, the LovDSR vault needs to borrow USDC. However, this action is not possible, resulting in the vault becoming frozen because current debt is 2500 USDC and it is equal to debt ceiling.

function _borrow(address borrower, address recipient,  BorrowerConfig storage borrowerConfig,  uint256 borrowAmount) private {
      debtToken.safeTransferFrom(address(idleStrategyManager), borrower, borrowAmount.scaleUp(_assetScalar));
      idleStrategyManager.withdraw(borrowAmount, recipient);

@2:      _refreshBorrowersInterestRate(borrower, borrowerConfig, true);
  }

@2: The validateUR parameter is true, so it will check whether the utilization ratio becomes larger than 100%.

function _calculateBorrowerInterestRate(
        address borrower, 
        BorrowerConfig storage borrowerConfig, 
        bool validateUR
    ) private view returns (uint96) {
    uint256 ur = _borrowerUtilisationRatio(borrower, borrowerConfig);
@3:    if (validateUR && ur > PRECISION) revert AboveMaxUtilisation(ur);

    return borrowerConfig.interestRateModel.calculateInterestRate(ur);
}

@3: will revert here

[frontier159]

User A repays 900 USDC to the LovDSR vault and this is possible due to the absence of access checking.

I think you mean User A repays 900 USDC to the OrigamiLendingClerk on behalf of the LovDSR vault?

This is by design (and why we have the borrower address as a parameter in the function)

If User A repays 900 USDC to the lending clerk, that is a donation, they cannot get economic gain from doing so - the only thing they've done is lower the current leverage of the vault temporarily.

Yes this raises the A/L, but the vault automations will just rebalance as necessary (normal operations) just like it would if a user makes a deposit into the lovDSR vault (which also raises the A/L potentially beyond the userALRange.ceiling)

The net result is actually a gain for lovDSR vault holders, since some of the vault debt has now been repaid.

[FCSE507]

@frontier159 , Could you please check the comment? Is this not a kind of DOS?

There is no restriction that debeCeiling which is lower than current debt can not be set.

[FCSE507]

@frontier159 , sure.

The LovDSR vault currently holds 3000 sDAI, with 300 sDAI contributed by users and 2700 sDAI obtained through borrowing USDC. The current A/L ratio is 10/9.

An elevated user updates the debt ceiling to 2400 USDC. This is possible because there is no restriction that debeCeiling which is lower than current debt can not be set.

function setBorrowerDebtCeiling(address borrower, uint256 newDebtCeiling) external override onlyElevatedAccess {
        BorrowerConfig storage borrowerConfig = _getBorrowerConfig(borrower);
        emit DebtCeilingUpdated(borrower, borrowerConfig.debtCeiling, newDebtCeiling);
        borrowerConfig.debtCeiling = newDebtCeiling;

        _checkpointDebtTokenBalances(borrower);
@1:        _refreshBorrowersInterestRate(borrower, borrowerConfig, false);
}
function _refreshBorrowersInterestRate(
    address borrower, 
    BorrowerConfig storage borrowerConfig, 
    bool validateUR
) private {
@2:    uint96 rate = _calculateCombinedInterestRate(borrower, borrowerConfig, validateUR);
    debtToken.setInterestRate(borrower, rate);
}

function _calculateCombinedInterestRate(
    address borrower, 
    BorrowerConfig storage borrowerConfig, 
    bool validateUR
) private view returns (uint96) {
@3:    uint96 _borrowerIR = _calculateBorrowerInterestRate(borrower, borrowerConfig, validateUR);

    uint96 _globalIR = _calculateGlobalInterestRate(validateUR);

    return _globalIR > _borrowerIR
        ? _globalIR
        : _borrowerIR;
}

function _calculateBorrowerInterestRate(
    address borrower, 
    BorrowerConfig storage borrowerConfig, 
    bool validateUR
) private view returns (uint96) {
    uint256 ur = _borrowerUtilisationRatio(borrower, borrowerConfig);
@4:    if (validateUR && ur > PRECISION) revert AboveMaxUtilisation(ur);

    return borrowerConfig.interestRateModel.calculateInterestRate(ur);
}

The calling order is @1->@2->@3->@4. And validateUR is false, so it will not be reverted. Now the debt ceiling of LovDSR becomes 2400.

A malicious user repays 300 and the A/L ratio becomes 3000/2400 = 1.25 and suppose that this ratio is outside of userALrange and rebalanceALrange.

In order to deposit or exit investment, we should decrease this ratio. But we can't do this because we are not be able to borrow USDC.

function _borrow(
      address borrower, 
      address recipient, 
      BorrowerConfig storage borrowerConfig, 
      uint256 borrowAmount
  ) private {
      if (borrowAmount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
      if (globalBorrowPaused) revert BorrowPaused();
      if (borrowerConfig.borrowPaused) revert BorrowPaused();

      // Check this borrow amount against the circuit breaker
      circuitBreakerProxy.preCheck(
          address(asset),
          msg.sender,
          borrowAmount
      );

      emit Borrow(borrower, recipient, borrowAmount);

      // Transfer the debt token amount from the idle strategy to the borrower
      // No allowances needed for debtToken, since only minters can transfer.
      debtToken.safeTransferFrom(address(idleStrategyManager), borrower, borrowAmount.scaleUp(_assetScalar));

      // Pull funds from idle strategy and send to recipient
      // If there aren't enough idle funds to cover this amount then it will revert.
      idleStrategyManager.withdraw(borrowAmount, recipient);

      // Refresh the borrower's interest rate.
      // Verify that both the global and borrower utilisation is <=100%
@5:      _refreshBorrowersInterestRate(borrower, borrowerConfig, true);
  }

@5: the validateUR paramter is true, so the transaction will be reverted in @4 because we already used the debt ceiling.

At this point, what can we do to resolve this DOS status?

Maybe I am missing something so would appreciate your explanation.

[frontier159]

If the lovToken's debt ceiling is updated such that it now has a higher debt than the ceiling, then it won't be able to borrow more anyway, so it cannot rebalanceDown(). It's worth noting that users can still exit the vault ok in this scenario.

If User A repays some of the debt on the lovToken's behalf, it doesn't change the fact that the lovToken cannot rebalanceDown(). If a separate user in the vault deposited more into lovToken (instead of User A repaying the debt), the vault would be in the exact same situation.

Importantly -- as part of restricting the debt ceiling, policy would be updating the userALRange and rebalanceALRange if it is not possible for the vault to borrow enough to get to the expected A/L. These are all elevated access controlled policy settings.

At most, the chance and impact of this (bad governance policy setting) is a low and would regardless need to be corrected with or without User A repaying the debt.

While we don't believe there is a vulnerability here (since User A could just deposit into the vault normally for the same result), we're willing to give a low as it may indeed be worth protecting this function as an extra precaution

[FCSE507]

Thanks for explanation.