sherlock-audit / 2023-03-notional-judging

12 stars 6 forks source link

xiaoming90 - No minimum borrow size check against secondary debts #212

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

No minimum borrow size check against secondary debts

Summary

Secondary debts were not checked against the minimum borrow size during exit, which could lead to accounts with insufficient debt becoming insolvent and the protocol incurring bad debts.

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-notional-0xleastwood/blob/main/contracts-v2/contracts/internal/vaults/VaultAccount.sol#L140

File: VaultAccount.sol
121:     function _setVaultAccount(
..SNIP..
130:         // An account must maintain a minimum borrow size in order to enter the vault. If the account
131:         // wants to exit under the minimum borrow size it must fully exit so that we do not have dust
132:         // accounts that become insolvent.
133:         if (
134:             vaultAccount.accountDebtUnderlying.neg() < vaultConfig.minAccountBorrowSize &&
135:             // During local currency liquidation and settlement, the min borrow check is skipped
136:             checkMinBorrow
137:         ) {
138:             // NOTE: use 1 to represent the minimum amount of vault shares due to rounding in the
139:             // vaultSharesToLiquidator calculation
140:             require(vaultAccount.accountDebtUnderlying == 0 || vaultAccount.vaultShares <= 1, "Min Borrow");
141:         }

A vault account has one primary debt (accountDebtUnderlying) and one or more secondary debts (accountDebtOne and accountDebtTwo).

When a vault account exits the vault, Notional will check that its primary debt (accountDebtUnderlying) meets the minimum borrow size requirement. If a vault account wants to exit under the minimum borrow size it must fully exit so that we do not have dust accounts that become insolvent. This check is being performed in Line 140 above.

However, this check is not performed against the secondary debts. As a result, it is possible that the secondary debts fall below the minimum borrow size after exiting.

Impact

Vault accounts with debt below the minimum borrow size are at risk of becoming insolvent, leaving the protocol with bad debts.

Code Snippet

https://github.com/sherlock-audit/2023-03-notional-0xleastwood/blob/main/contracts-v2/contracts/internal/vaults/VaultAccount.sol#L140

Tool used

Manual Review

Recommendation

Consider performing a similar check against the secondary debts (accountDebtOne and accountDebtTwo) within the _setVaultAccount function to ensure they do not fall below the minimum borrow size.

jeffywu commented 1 year ago

Valid issue

jeffywu commented 1 year ago

Upon further review, it seems like the issue is invalid. Minimum borrow checks are applied every time debt changes.