Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Yearn/YearnVaultStorage.sol#L39-L45

Vulnerability details

Impact

Yearn Curve vault can be removed without withdrawing all deposited tokens. Causing these token to be locked forever unless an owner is added this vault back.

Proof of Concept

  function removeVault(address vault) external onlyOwner {
      require(vaults[vault] != address(0), "YVS: NON_EXISTENT_VAULT");
      delete vaults[vault];
      emit VaultRemoved(vault);
  }

Once vault is deleted, it can't be deposit or withdraw using operator anymore. as it will be reverted with "YCVO: INVALID_VAULT" as shown below

    function withdrawETH(
        address vault,
        uint256 amount,
        uint256 minAmountOut
    ) external payable returns (uint256[] memory amounts, address[] memory tokens) {
        require(amount != 0, "YCVO: INVALID_AMOUNT");

        (address pool, uint96 poolCoinAmount, address lpToken) = operatorStorage.vaults(vault);
        require(pool != address(0), "YCVO: INVALID_VAULT");

deleted vault -> operatorStorage.vaults(vault) = address(0) -> revered with "YCVO: INVALID_VAULT"

Tools Used

Manual

Recommended Mitigation Steps

Before removing vault, check for balance of the vault, it should be zero
You should create a separate function to only disable the deposit

  // TODO: Add operator variable and set it in constructor

  /// @notice Remove a beefy vault
  /// @param vault The vault address to remove
  function removeVault(address vault) external onlyOwner {
      require(vaults[vault] != address(0), "BVS: NON_EXISTENT_VAULT");
      // TODO: Check deposited but still not withdraw balance in vault here
      delete vaults[vault];
      emit VaultRemoved(vault);
  }

  // TODO: Create a separate function to only disable the deposit.
  function disableVaultDeposit(address vault) external onlyOwner { ... }

  // TODO: Add more require to deposit(...) to prevent deposit to vault that is deposit disabled.

code-423n4 / 2022-06-nested-findings

Yearn Curve vault can be removed without withdrawing all deposited tokens. Causing these token to be locked forever unless an owner is added this vault back. #79