Description

The function _removeVaults is a helper function that removes vaults from the vaultList. While removing a vault from the middle of the array, it is intended to replace the vault at the last index with the vault to be removed and pop the last vault.

The index of the last element of the array should be removeCount - 1 (where removeCount = vaults.length), but the function is using the last element as removeCount --- due to which it will revert because it would access element out-of-bounds of the array.

Shown below is the relevant part of the code:

function _removeVaults(
    address[] memory vaults
) internal returns (address[] memory newVaultList) {
    //...
    } else {
        if (vaults.length > 1) {
            vaults[j] = vaults[removeCount];
            delete vaults[removeCount];
        } else delete vaults[j];
        removeCount--;
    }
    //...

Impact

The _removeVaults function would revert in certain cases.

Recommendations

Use the correct last array index removeCount - 1 instead of removeCount:

function _removeVaults(
    address[] memory vaults
) internal returns (address[] memory newVaultList) {
    //...
    } else {
        if (vaults.length > 1) {
-           vaults[j] = vaults[removeCount];
-           delete vaults[removeCount];
+           vaults[j] = vaults[removeCount - 1];
+           delete vaults[removeCount - 1];
    } else delete vaults[j];
    removeCount--;
    }
    //...

Remediation

The issue was fixed in commit fd2a6f3↗.