joestakey - Notional Governors can use `reduceMaxBorrowCapacity` on a vault to increase `maxBorrowCapacity`, which can grief users of the vault.

[sherlock-admin]

joestakey

medium

Notional Governors can use reduceMaxBorrowCapacity on a vault to increase maxBorrowCapacity, which can grief users of the vault.

Summary

Notional Governors can increase the risk of a vault at any time by using reduceMaxBorrowCapacity to increase maxBorrowCapacity, which can lead to insolvency of users.

Vulnerability Detail

VaultAction.reduceMaxBorrowCapacity() is meant to

reduce the max borrow capacity on the vault and force
the redemption of strategy tokens to cash to reduce the overall risk of the vault.
This method is intended to be used in emergencies to mitigate insolvency risk. The effect
of this method will mean that the overall max borrow capacity is reduced, the total used
capacity will be unchanged (redeemStrategyTokensToCash does not do any lending to reduce
the outstanding fCash), and accounts will be locked out of entering the maturity which was
targeted by this method

The function calls VaultConfiguration.setMaxBorrowCapacity, which sets the new maxBorrowCapacity without any check on the new value being written.

This means it is technically possible for Notional governors to actually increase the maxBorrowCapacity.

Impact

As detailed in the function comment Other maturities for that vault may still be entered depending on whether or not the vault is above or below the max vault borrow capacity. This would effectively increase the risk on users having entered the vaults on these other maturities:

The higher borrowing capacity means new accounts can keep borrowing, increasing here totalVaultShares.

• VaultState.getCashValueOfShare() returns assetCashValue which will be lower due to getPoolShare return values being lower as vaultState.totalVaultShares is higher
• In VaultConfiguration.calculateCollateralRatio(), vaultShareValue is lower , which also results in netAssetValue being lower. If too low:
• either the account is insolvent
• if high enough to be > 0, the return value collateralRatio will trigger this check to pass, meaning that the account can be liquidated.

Code Snippet

https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAction.sol#L103

Tool used

Manual Review

Recommendation

VaultConfiguration.setMaxBorrowCapacity should check the new maxBorrowCapacity to ensure it is not greater than the current one. https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/internal/vaults/VaultConfiguration.sol#L224

        VaultBorrowCapacityStorage storage cap = LibStorage.getVaultBorrowCapacity()[vault][currencyId];
+       require(cap.maxBorrowCapacity > maxBorrowCapacity, "invalid new maxBorrowCapacity");
        cap.maxBorrowCapacity = maxBorrowCapacity;

[jeffywu]

Max borrow capacities can increase and they will not dilute vault shares. I consider this issue to be invalid.

[T-Woodward]

Agreed. Even though this specific function isn't meant to be used to increase vault capacity, it doesn't matter that it can be because the same owner has a different function available to them that is meant to be used to do that.