Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Beefy/BeefyVaultStorage.sol#L34-L38

Vulnerability details

Impact

Beefy 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), "BVS: 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 "BVO: INVALID_VAULT" as shown below

    function withdraw(address vault, uint256 amount)
        external
        returns (uint256[] memory amounts, address[] memory tokens)
    {
        require(amount != 0, "BVO: INVALID_AMOUNT");
        IERC20 token = IERC20(operatorStorage.vaults(vault));
        require(address(token) != address(0), "BVO: INVALID_VAULT");

deleted vault -> token = IERC20(operatorStorage.vaults(vault)); = address(0) -> revered with "BVO: 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");
        require(IERC20(vault).balanceOf(operator) == 0, "BVS: NOT_ZERO_BALANCE");
        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

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