Initialization of Cross Currency Vault Can Be Front-runned

Summary

The initialization of the Cross Currency Vault can be front-runned due to the lack of access control.

Vulnerability Detail

onlyNotionalOwner has been implemented on Balancer-related vaults to ensure that the vault initialization cannot be front-runned by malicious users.

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

File: MetaStable2TokenAuraVault.sol
44:     function initialize(InitParams calldata params)
45:         external
46:         initializer
47:         onlyNotionalOwner
48:     {
49:         __INIT_VAULT(params.name, params.borrowCurrencyId);
50:         BalancerVaultStorage.setStrategyVaultSettings(
51:             params.settings, MAX_ORACLE_QUERY_WINDOW, BalancerConstants.VAULT_PERCENT_BASIS
52:         );
53:         _twoTokenPoolContext()._approveBalancerTokens(address(_auraStakingContext().auraBooster));
54:     }

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

File: Boosted3TokenAuraVault.sol
42:     function initialize(InitParams calldata params)
43:         external
44:         initializer
45:         onlyNotionalOwner
46:     {
47:         __INIT_VAULT(params.name, params.borrowCurrencyId);
48:         // 3 token vaults do not use the Balancer oracle
49:         BalancerVaultStorage.setStrategyVaultSettings(
50:             params.settings, 
51:             0, // Max Balancer oracle window size
52:             0  // Balancer oracle weight
53:         );
54: 
55:         (
56:             /* address[] memory tokens */,
57:             uint256[] memory balances,
58:             /* uint256 lastChangeBlock */
59:         ) = Deployments.BALANCER_VAULT.getPoolTokens(BALANCER_POOL_ID);
60: 
61:         _threeTokenPoolContext(balances)._approveBalancerTokens(address(_auraStakingContext().auraBooster));
62:     }

However, this access control was not consistently applied to the Cross Currency vault. As shown below, the CrossCurrencyfCashVault.initialize function lacks the onlyNotionalOwner modifier. Thus, anyone can trigger the CrossCurrencyfCashVault.initialize function as long as it has not been called before.

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

File: CrossCurrencyfCashVault.sol
079:     function initialize(
080:         string memory name_,
081:         uint16 borrowCurrencyId_,
082:         uint16 lendCurrencyId_,
083:         uint64 settlementSlippageLimit_
084:     ) external initializer {
085:         __INIT_VAULT(name_, borrowCurrencyId_);
086: 
087:         LEND_CURRENCY_ID = lendCurrencyId_;
088:         (
089:             Token memory assetToken,
090:             Token memory underlyingToken,
091:             /* ETHRate memory ethRate */,
092:             /* AssetRateParameters memory assetRate */
093:         ) = NOTIONAL.getCurrencyAndRates(lendCurrencyId_);
094: 
095:         IERC20 tokenAddress = assetToken.tokenType == TokenType.NonMintable ?
096:             IERC20(assetToken.tokenAddress) : IERC20(underlyingToken.tokenAddress);
097:         LEND_UNDERLYING_TOKEN = tokenAddress;
098: 
099:         // Allow Notional to pull the lend underlying currency
100:         tokenAddress.approve(address(NOTIONAL), type(uint256).max);
101: 
102:         // This value cannot be greater than 1e18
103:         require(settlementSlippageLimit_ < SETTLEMENT_SLIPPAGE_PRECISION);
104:         settlementSlippageLimit = settlementSlippageLimit_;
105:     }

Impact

A malicious attacker could monitor the Ethereum blockchain for bytecode that matches the CrossCurrencyfCashVault contract and front-run the initialize() transaction to configure the vaults in a manner that will benefit the malicious user. This can be repeated as a Denial Of Service (DOS) type of attack, effectively preventing Notional’s contract deployment, leading to unrecoverable gas expenses

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/MetaStable2TokenAuraVault.sol#L44 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/Boosted3TokenAuraVault.sol#L42 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/CrossCurrencyfCashVault.sol#L79

Tool used

Manual Review

Recommendation

It is recommended to implement access control on all the initialize functions to ensure that only the Notional admin can initialize and configure the vaults.

function initialize(
    string memory name_,
    uint16 borrowCurrencyId_,
    uint16 lendCurrencyId_,
    uint64 settlementSlippageLimit_
- ) external initializer {
+ ) external initializer onlyNotionalOwner {
    __INIT_VAULT(name_, borrowCurrencyId_);

    LEND_CURRENCY_ID = lendCurrencyId_;
    (
        Token memory assetToken,
        Token memory underlyingToken,
        /* ETHRate memory ethRate */,
        /* AssetRateParameters memory assetRate */
    ) = NOTIONAL.getCurrencyAndRates(lendCurrencyId_);

    IERC20 tokenAddress = assetToken.tokenType == TokenType.NonMintable ?
        IERC20(assetToken.tokenAddress) : IERC20(underlyingToken.tokenAddress);
    LEND_UNDERLYING_TOKEN = tokenAddress;

    // Allow Notional to pull the lend underlying currency
    tokenAddress.approve(address(NOTIONAL), type(uint256).max);

    // This value cannot be greater than 1e18
    require(settlementSlippageLimit_ < SETTLEMENT_SLIPPAGE_PRECISION);
    settlementSlippageLimit = settlementSlippageLimit_;
}

sherlock-audit / 2022-09-notional-judging

xiaoming90 - Initialization of Cross Currency Vault Can Be Front-runned #61