xiaoming90 - Users Can Enter Balancer Vault Within Settlement Period

[sherlock-admin]

xiaoming90

medium

Users Can Enter Balancer Vault Within Settlement Period

Summary

Users are not allowed to enter the vault within the settlement windows. However, it was observed that it is possible for a user to enter the vault within the settlement window.

Vulnerability Detail

For Balancer-related vaults, assume that the vault will mature on Day 10, then the settlement window is Maturity date (day 10) - 3 days settlement period, which means that the settlement window is from Day 8 to Day 10. As per the comment on Line 70 below, no one is allowed to enter the vault within the settlement window.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/Boosted3TokenAuraVault.sol#L64

File: Boosted3TokenAuraVault.sol
64:     function _depositFromNotional(
65:         address /* account */,
66:         uint256 deposit,
67:         uint256 maturity,
68:         bytes calldata data
69:     ) internal override returns (uint256 strategyTokensMinted) {
70:         // Entering the vault is not allowed within the settlement window
71:         DepositParams memory params = abi.decode(data, (DepositParams));
72:         Boosted3TokenAuraStrategyContext memory context = _strategyContext();
73: 
74:         strategyTokensMinted = context.poolContext._deposit({
75:             strategyContext: context.baseStrategy,
76:             stakingContext: context.stakingContext,
77:             oracleContext: context.oracleContext, 
78:             deposit: deposit,
79:             minBPT: params.minBPT
80:         });
81:     }

However, per the check at Line 55 below, a user can enter the vault as long as it has not passed the maturity day (Day 10) yet. Therefore, it is possible for a user to enter the vault within the settlement window.

https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/external/actions/VaultAccountAction.sol#L34

File: VaultAccountAction.sol
34:     function enterVault(
35:         address account,
36:         address vault,
37:         uint256 depositAmountExternal,
38:         uint256 maturity,
39:         uint256 fCash,
40:         uint32 maxBorrowRate,
41:         bytes calldata vaultData
42:     ) external payable override nonReentrant returns (uint256 strategyTokensAdded) { 
43:         // Ensure that system level accounts cannot enter vaults
44:         requireValidAccount(account);
45:         VaultConfig memory vaultConfig = VaultConfiguration.getVaultConfigStateful(vault);
46:         vaultConfig.authorizeCaller(account, VaultConfiguration.ONLY_VAULT_ENTRY);
47: 
48:         // If the vault allows further re-entrancy then set the status back to the default
49:         if (vaultConfig.getFlag(VaultConfiguration.ALLOW_REENTRANCY)) {
50:             reentrancyStatus = _NOT_ENTERED;
51:         }
52: 
53:         // Vaults cannot be entered if they are paused or matured
54:         require(vaultConfig.getFlag(VaultConfiguration.ENABLED), "Cannot Enter");
55:         require(block.timestamp < maturity, "Cannot Enter");
56:         VaultAccount memory vaultAccount = VaultAccountLib.getVaultAccount(account, vault);
57: 
58:         uint256 strategyTokens;
59:         if (vaultAccount.maturity != 0 && vaultAccount.maturity <= block.timestamp) {
60:             // These strategy tokens will be transferred to the new maturity
61:             strategyTokens = vaultAccount.settleVaultAccount(vaultConfig, block.timestamp);
62:         }
63: 
64:         // Deposits some amount of underlying tokens into the vault directly to serve as additional collateral when
65:         // entering the vault.
66:         uint256 additionalUnderlyingExternal = vaultConfig.transferUnderlyingToVaultDirect(account, depositAmountExternal);
67:         strategyTokensAdded = vaultAccount.borrowAndEnterVault(
68:             vaultConfig, maturity, fCash, maxBorrowRate, vaultData, strategyTokens, additionalUnderlyingExternal
69:         );
70: 
71:         emit VaultEnterPosition(vault, account, maturity, fCash);
72:     }

Impact

Allowing users to enter a vault within the settlement windows essentially break the specification/requirement of allowing no one to enter the vault within the settlement window. It will cause internal accounting issues within the vault if this happens.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/Boosted3TokenAuraVault.sol#L64 https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/external/actions/VaultAccountAction.sol#L34

Tool used

Manual Review

Recommendation

It is recommended that no one is allowed to enter the vault within the settlement window under any circumstance. The settlement window is from maturity - SETTLEMENT_PERIOD_IN_SECONDS to maturity.

function _depositFromNotional(
    address /* account */,
    uint256 deposit,
    uint256 maturity,
    bytes calldata data
) internal override returns (uint256 strategyTokensMinted) {
    // Entering the vault is not allowed within the settlement window
+   if (block.timestamp >= maturity - SETTLEMENT_PERIOD_IN_SECONDS) {
+       revert Errors.CannotEnterWithinSettlementWindow();
+   }
+    
    DepositParams memory params = abi.decode(data, (DepositParams));
    Boosted3TokenAuraStrategyContext memory context = _strategyContext();

    strategyTokensMinted = context.poolContext._deposit({
        strategyContext: context.baseStrategy,
        stakingContext: context.stakingContext,
        oracleContext: context.oracleContext, 
        deposit: deposit,
        minBPT: params.minBPT
    });
}

[jeffywu]

It's not clear what internal account issues this would cause? Users cannot enter the settlement window once a single settlement has occurred. I would consider this issue invalid unless a specific internal account issue can be identified.

[T-Woodward]

Agreed. While this may be semantically un-intuitive, it doesn't cause any real issue because users can't enter the vault after settlement proceedings have actually begun and that's what matters.