xiaoming90 - Possible to liquidate past the debt outstanding above the min borrow without liquidating the entire debt outstanding

[sherlock-admin]

xiaoming90

high

Possible to liquidate past the debt outstanding above the min borrow without liquidating the entire debt outstanding

Summary

It is possible to liquidate past the debt outstanding above the min borrow without liquidating the entire debt outstanding. Thus, leaving accounts with small debt that are not profitable to unwind if it needs to liquidate.

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultValuation.sol#L251

File: VaultValuation.sol
250:         // NOTE: deposit amount is always positive in this method
251:         if (depositUnderlyingInternal < maxLiquidatorDepositLocal) {
252:             // If liquidating past the debt outstanding above the min borrow, then the entire
253:             // debt outstanding must be liquidated.
254: 
255:             // (debtOutstanding - depositAmountUnderlying) is the post liquidation debt. As an
256:             // edge condition, when debt outstanding is discounted to present value, the account
257:             // may be liquidated to zero while their debt outstanding is still greater than the
258:             // min borrow size (which is normally enforced in notional terms -- i.e. non present
259:             // value). Resolving this would require additional complexity for not much gain. An
260:             // account within 20% of the minBorrowSize in a vault that has fCash discounting enabled
261:             // may experience a full liquidation as a result.
262:             require(
263:                 h.debtOutstanding[currencyIndex].sub(depositUnderlyingInternal) < minBorrowSize,
264:                 "Must Liquidate All Debt"
265:             );

• depositUnderlyingInternal is always a positive value (Refer to comment on Line 250) that represents the amount of underlying deposited by the liquidator
• h.debtOutstanding[currencyIndex] is always a negative value representing debt outstanding of a specific currency in a vault account
• minBorrowSize is always a positive value that represents the minimal borrow size of a specific currency (It is stored as uint32 in storage)

If liquidating past the debt outstanding above the min borrow, then the entire debt outstanding must be liquidated.

Assume the following scenario:

• depositUnderlyingInternal = 70 USDC
• h.debtOutstanding[currencyIndex] = -100 USDC
• minBorrowSize = 50 USDC

If the liquidation is successful, the vault account should be left with -30 USDC debt outstanding because 70 USDC has been paid off by the liquidator. However, this should not happen under normal circumstances because the debt outstanding (-30) does not meet the minimal borrow size of 50 USDC and the liquidation should revert/fail.

The following piece of validation logic attempts to ensure that all outstanding debt is liquidated if post-liquidation debt does not meet the minimal borrowing size.

require(
    h.debtOutstanding[currencyIndex].sub(depositUnderlyingInternal) < minBorrowSize,
    "Must Liquidate All Debt"
);

Plugging in the values from our scenario to verify if the code will revert if the debt outstanding does not meet the minimal borrow size.

require(
	(-100 USDC - 70 USDC) < 50 USDC
);
===>
require(
	(-170 USDC) < 50 USDC
);
===>
require(true) // no revert

The above shows that it is possible for someone to liquidate past the debt outstanding above the min borrow without liquidating the entire debt outstanding. This shows that the math formula in the code is incorrect and not working as intended.

Impact

A liquidation can bring an account below the minimum debt. Accounts smaller than the minimum debt are not profitable to unwind if it needs to liquidate (Reference)

As a result, liquidators are not incentivized to liquidate those undercollateralized positions. This might leave the protocol with bad debts, potentially leading to insolvency if the bad debts accumulate.

Code Snippet

https://github.com/sherlock-audit/2023-03-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultValuation.sol#L251

Tool used

Manual Review

Recommendation

Update the formula to as follows:

require(
-   h.debtOutstanding[currencyIndex].sub(depositUnderlyingInternal) < minBorrowSize,
+   h.debtOutstanding[currencyIndex].neg().sub(depositUnderlyingInternal) > minBorrowSize,
    "Must Liquidate All Debt"
);

Plugging in the values from our scenario again to verify if the code will revert if the debt outstanding does not meet the minimal borrow size.

require(
	((-100 USDC).neg() - 70 USDC) > 50 USDC
);
===>
require(
	(100 USDC - 70 USDC) > 50 USDC
);
===>
require(
	(30 USDC) > 50 USDC
);
===>
require(false) // revert

The above will trigger a revert as expected when the debt outstanding does not meet the minimal borrow size.

[jeffywu]

Fixed: notional-finance/contracts-v2#132

[xiaoming9090]

@0xleastwood + @xiaoming9090 : Verified. Fixed in PR notional-finance/contracts-v2#132