MetaStable2TokenAuraVault allows only up to 1bp weight for Balancer TWAP oracle

Summary

MetaStable2TokenAuraVault's Vault settings are initialized and set with Balancer TWAP oracle weight limit hard coded to BalancerConstants.VAULT_PERCENT_BASIS, which is only 1bp of the total weight, which means that Balancer TWAP do not take any meaningful part in Vault's pricing.

Vulnerability Detail

_getOraclePairPrice() output is the weighted average of Chainlink and Balancer reported values. While such setup is constructed to enhance the stability of the resulting price feed, now it's incorrectly initialized in such a way that Balancer TWAP oracle's share will be at most 1bp, i.e. it has almost no influence on the price and price feed stability is basically the same as one of Chainlink oracle.

Impact

MetaStable2TokenAuraVault strategy will be priced off at least 0.9999 of Chanlink's price and at most 0.0001 of Balancer's TWAP. This way any Chanlink malfunctions and volatility spikes will be almost fully translated to the MetaStable2TokenAuraVault pricing, while Balancer's TWAP is being ignored.

Net impact is the user losses in the event of Chainlink price feed erroneous values (typical mispricing surfaces: liquidation of healthy accounts, prohibition of liquidation of the unhealthy ones).

As this is the condition for the net loss for the users to occur despite clear technical misconfiguration, setting the severity to be medium.

Code Snippet

MetaStable2TokenAuraVault requires that balancerOracleWeight <= BalancerConstants.VAULT_PERCENT_BASIS, i.e. VAULT_PERCENT_BASIS is set as Balancer TWAP oracle weight limit:

initialize:

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

    function initialize(InitParams calldata params)
        external
        initializer
        onlyNotionalOwner
    {
        __INIT_VAULT(params.name, params.borrowCurrencyId);
        BalancerVaultStorage.setStrategyVaultSettings(
            params.settings, MAX_ORACLE_QUERY_WINDOW, BalancerConstants.VAULT_PERCENT_BASIS
        );

setStrategyVaultSettings:

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

    /// @notice Updates the vault settings
    /// @param settings vault settings
    function setStrategyVaultSettings(StrategyVaultSettings calldata settings)
        external
        onlyNotionalOwner
    {
        BalancerVaultStorage.setStrategyVaultSettings(
            settings, MAX_ORACLE_QUERY_WINDOW, BalancerConstants.VAULT_PERCENT_BASIS
        );
    }

In the same time the weight is then calculated out of BALANCER_ORACLE_WEIGHT_PRECISION = 1e8:

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/BalancerConstants.sol#L7-L12

    uint256 internal constant BALANCER_ORACLE_WEIGHT_PRECISION = 1e8;
    uint32 internal constant SLIPPAGE_LIMIT_PRECISION = 1e8;

    /// @notice Precision for all percentages used by the vault
    /// 1e4 = 100% (i.e. maxBalancerPoolShare)
    uint16 internal constant VAULT_PERCENT_BASIS = 1e4;

I.e. _getOraclePairPrice() logic is based on an assumption that balancerOracleWeight is a part of the BalancerConstants.BALANCER_ORACLE_WEIGHT_PRECISION:

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/TwoTokenPoolUtils.sol#L66-L114

    /// @notice Gets the oracle price pair price between two tokens using a weighted
    /// average between a chainlink oracle and the balancer TWAP oracle.
    /// @param poolContext oracle context variables
    /// @param oracleContext oracle context variables
    /// @param tradingModule address of the trading module
    /// @return oraclePairPrice oracle price for the pair in 18 decimals
    function _getOraclePairPrice(
        TwoTokenPoolContext memory poolContext,
        OracleContext memory oracleContext, 
        ITradingModule tradingModule
    ) internal view returns (uint256 oraclePairPrice) {
        // NOTE: this balancer price is denominated in 18 decimal places
        uint256 balancerWeightedPrice;
        if (oracleContext.balancerOracleWeight > 0) {
            uint256 balancerPrice = BalancerUtils._getTimeWeightedOraclePrice(
                address(poolContext.basePool.pool),
                IPriceOracle.Variable.PAIR_PRICE,
                oracleContext.oracleWindowInSeconds
            );

            if (poolContext.primaryIndex == 1) {
                // If the primary index is the second token, we need to invert
                // the balancer price.
                balancerPrice = BalancerConstants.BALANCER_PRECISION_SQUARED / balancerPrice;
            }

            balancerWeightedPrice = balancerPrice * oracleContext.balancerOracleWeight;
        }

        uint256 chainlinkWeightedPrice;
        if (oracleContext.balancerOracleWeight < BalancerConstants.BALANCER_ORACLE_WEIGHT_PRECISION) {
            (int256 rate, int256 decimals) = tradingModule.getOraclePrice(
                poolContext.primaryToken, poolContext.secondaryToken
            );
            require(rate > 0);
            require(decimals >= 0);

            if (uint256(decimals) != BalancerConstants.BALANCER_PRECISION) {
                rate = (rate * int256(BalancerConstants.BALANCER_PRECISION)) / decimals;
            }

            // No overflow in rate conversion, checked above
            chainlinkWeightedPrice = uint256(rate) * 
                (BalancerConstants.BALANCER_ORACLE_WEIGHT_PRECISION - oracleContext.balancerOracleWeight);
        }

        oraclePairPrice = (balancerWeightedPrice + chainlinkWeightedPrice) / 
            BalancerConstants.BALANCER_ORACLE_WEIGHT_PRECISION;
    }

This way requiring that balancerOracleWeight <= BalancerConstants.VAULT_PERCENT_BASIS means that no more than 1bp, VAULT_PERCENT_BASIS / BALANCER_ORACLE_WEIGHT_PRECISION = 1e-4, of the _getOraclePairPrice's oraclePairPrice will consists of Balancer TWAP oracle.

This doesn't fit any logic, i.e. if Balancer TWAP oracle price shouldn't be used the limit should be zero, as it's done in the Boosted3TokenAuraVault's case:

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

    /// @notice Updates the vault settings
    /// @param settings vault settings
    function setStrategyVaultSettings(StrategyVaultSettings calldata settings)
        external
        onlyNotionalOwner
    {
        // 3 token vaults do not use the Balancer oracle
        BalancerVaultStorage.setStrategyVaultSettings(
            settings, 
            0, // Max Balancer oracle window size
            0  // Balancer oracle weight
        );
    }

If it should be used, the limit need to be set higher to allow some meaningful value.

Tool used

Manual Review

Recommendation

Consider updating the limit so a non-trivial weight can be set for Balancer oracle, for example: