sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Users Unable To Enter Vault After Emergency Settlement #54

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

Users Unable To Enter Vault After Emergency Settlement

Summary

Users will not be able to enter a vault after an emergency settlement has occurred.

Vulnerability Detail

Note 1: I have clarified with the sponsor and noted that they are already aware of the fact that no users can enter the vault after an emergency settlement and they further clarified that the users could still exit the vault by taking their proportional share of cash and strategy tokens, and this is their current design at this stage. However, for completeness' sake and clarity, I will document this limitation observed during the audit in this report as this was not explicitly documented anywhere else.

Note 2: This issue affects MetaStable2 and Boosted3 balancer leverage vaults

There might be a case where the vault owns too much of the Balancer pool. The purpose of an emergency settlement is to allow the vault to sell off some of the holdings of the Balancer pool to reduce the vault's holding to bring it back to a healthy ratio.

Based on the current design, the users can only enter the vault if the vault has no asset cash.

https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultState.sol#L207

File: VaultState.sol
206:     /// @param vaultData calldata to pass to the vault
207:     function enterMaturity(
208:         VaultState memory vaultState,
209:         VaultAccount memory vaultAccount,
210:         VaultConfig memory vaultConfig,
211:         uint256 strategyTokenDeposit,
212:         uint256 additionalUnderlyingExternal,
213:         bytes calldata vaultData
214:     ) internal returns (uint256 strategyTokensAdded) {
215:         // If the vault state is holding asset cash this would mean that there is some sort of emergency de-risking
216:         // event or the vault is in the process of settling debts. In both cases, we do not allow accounts to enter
217:         // the vault.
218:         require(vaultState.totalAssetCash == 0);
219:         // An account cannot enter a vault with a negative temp cash balance.  This can happen during roll vault where
220:         // an insufficient amount is borrowed to repay its previous maturity debt.
221:         require(vaultAccount.tempCashBalance >= 0);

However, after an emergency settlement, there will be some asset cash in the vault.

Proof-of-Concept

The following PoC shows that after an emergency settlement, the users will not be able to enter the vault again.

Before the emergency settlement, the script asserts that there is no asset cash (assert vaultState["totalAssetCash"] == 0). After the emergency settlement, the script assets that there is some asset cash in the vault (assert vaultState["totalAssetCash"] > 0). At the end of the script, it assets that the user's attempt to enter the vault has reverted.

import math
import time
import pytest
import brownie
from brownie import accounts
from brownie.network.state import Chain
from tests.fixtures import *
from tests.balancer.helpers import enterMaturity
from scripts.common import (
    get_updated_vault_settings, 
    get_dynamic_trade_params,
    get_univ3_single_data,
    get_redeem_params,
    DEX_ID,
    TRADE_TYPE
)

chain = Chain()

def test_emergency_single_maturity(StratStableETHstETH):
    (env, vault) = StratStableETHstETH
    primaryBorrowAmount = 5e8
    depositAmount = 10e18
    maturity = enterMaturity(env, vault, 1, 0, depositAmount, primaryBorrowAmount, accounts[0])
    settings = vault.getStrategyContext()["baseStrategy"]["vaultSettings"]
    oldMaxBalancerPoolShare = settings["maxBalancerPoolShare"]
    vault.setStrategyVaultSettings(get_updated_vault_settings(settings, maxBalancerPoolShare=0), {"from": env.notional.owner()})
    redeemParams = get_redeem_params(
        0, 0, get_dynamic_trade_params(DEX_ID["CURVE"], TRADE_TYPE["EXACT_IN_SINGLE"], 5e6, True, bytes(0))
    )

    vaultState = env.notional.getVaultState(vault.address, maturity)
    assert vaultState["totalAssetCash"] == 0

    vault.settleVaultEmergency(maturity, redeemParams, {"from": accounts[1]})

    vaultState = env.notional.getVaultState(vault.address, maturity)
    assert vaultState["totalStrategyTokens"] == 0
    assert vaultState["totalAssetCash"] > 0

    vault.setStrategyVaultSettings(get_updated_vault_settings(settings, maxBalancerPoolShare=oldMaxBalancerPoolShare), {"from": env.notional.owner()})
    primaryBorrowAmount = 5e8
    depositAmount = 10e18
    with brownie.reverts():
        maturity = enterMaturity(env, vault, 1, 0, depositAmount, primaryBorrowAmount, accounts[0])
❯ brownie test tests/balancer/settlement/test_settlement_stable_eth_steth.py 
Brownie v1.18.1 - Python development framework for Ethereum

===================================================== test session starts ======================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
plugins: eth-brownie-1.18.1, hypothesis-6.27.3, forked-1.4.0, xdist-1.34.0, web3-5.27.0
collected 1 item                                                                                                               

Launching 'npx hardhat node --port 8545 --fork https://eth-mainnet.g.alchemy.com/v2/DRXs2eoKsWyJxZuPZkSoN5SkS2Og-hEN --fork-block-number 15447569'...

tests/balancer/settlement/test_settlement_stable_eth_steth.py .                                                          [100%]

====================================================== 1 passed in 11.33s ======================================================
Terminating local RPC client...

Impact

After an emergency settlement, the vault will be "locked up". Users will not be able to enter the vault, and they are only allowed to exit the vault.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultState.sol#L207

Tool used

Manual Review

Recommendation

It is recommended to allow users to enter the vault after the emergency settlement. Alternatively, document the impact of an emergency settlement so that the users will be aware of any side effects of an emergency settlement.