xiaoming90 - `CrossCurrencyfCashVault` Cannot Be Upgraded

[sherlock-admin]

xiaoming90

medium

CrossCurrencyfCashVault Cannot Be Upgraded

Summary

CrossCurrencyfCashVault cannot be upgraded as it is missing the authorize upgrade method.

Vulnerability Detail

The Cross Currency Vault is expected to be upgradeable as:

• This vault is similar to the other vaults (Boosted3TokenAuraVault and MetaStable2TokenAuraVault) provided by Notional that are upgradeable by default.
• The BaseStrategyVault has configured the storage gaps uint256[45] private __gap for upgrading purposes
• Clarified with the sponsor and noted that Cross Currency Vault should be upgradeable

CrossCurrencyfCashVault inherits from BaseStrategyVault. However, the BaseStrategyVault forget to inherit Openzepplin's UUPSUpgradeable contract. Therefore, it is missing the authorize upgrade method, and the contract cannot be upgraded.

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

abstract contract BaseStrategyVault is Initializable, IStrategyVault {
    using TokenUtils for IERC20;
    using TradeHandler for Trade;

    /// @notice Hardcoded on the implementation contract during deployment
    NotionalProxy public immutable NOTIONAL;
    ITradingModule public immutable TRADING_MODULE;
    uint8 constant internal INTERNAL_TOKEN_DECIMALS = 8;

    ..SNIP..

    // Storage gap for future potential upgrades
    uint256[45] private __gap;
 }

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

contract CrossCurrencyfCashVault is BaseStrategyVault {
    using TypeConvert for uint256;
    using TypeConvert for int256;

    uint256 public constant SETTLEMENT_SLIPPAGE_PRECISION = 1e18;

    struct DepositParams {
        // Minimum purchase amount of the lend underlying token, this is
        // based on the deposit + borrowed amount and must be set to a non-zero
        // value to establish a slippage limit.
        uint256 minPurchaseAmount;
        // Minimum annualized lending rate, can be set to zero for no slippage limit
        uint32 minLendRate;
        // ID of the desired DEX to trade on, _depositFromNotional will always trade
        // using an EXACT_IN_SINGLE trade which is supported by all DEXes
        uint16 dexId;
        // Exchange data depending on the selected dexId
        ..SNIP..

Impact

If a critical bug is discovered within the Cross Currency Vault after launching that causes a loss of assets, the vault cannot be upgraded unlike the other balancer-related vaults to fix the bugs. All assets within the vault will be lost

Code Snippet

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

Tool used

Manual Review

Recommendation

It is recommended to Inherit Openzepplin's UUPSUpgradeable contract and implement the missing authorize upgrade method.

- abstract contract BaseStrategyVault is Initializable, IStrategyVault {
+ abstract contract BaseStrategyVault is Initializable, IStrategyVault, UUPSUpgradeable {
    using TokenUtils for IERC20;
    using TradeHandler for Trade;

    /// @notice Hardcoded on the implementation contract during deployment
    NotionalProxy public immutable NOTIONAL;
    ITradingModule public immutable TRADING_MODULE;
    uint8 constant internal INTERNAL_TOKEN_DECIMALS = 8;

    ..SNIP..

+    function _authorizeUpgrade(
+        address /* newImplementation */
+    ) internal override onlyNotionalOwner {}    

    // Storage gap for future potential upgrades
    uint256[45] private __gap;
 }

[jeffywu]

Valid issue, the vault is missing the proper method.