Risk of reward tokens being sold by malicious users under certain conditions
Due to the lack of validation of the selling token within the Curve adaptors, there is a risk that the reward tokens or Convex deposit tokens of the vault being sold by malicious users under certain conditions (e.g. if reward tokens equal to primary/secondary tokens OR a new exploit is found in other parts of the code).\\nFor a `EXACT_IN_SINGLE` trade within the Curve adaptors, the `from` and `to` addresses of the `exchange` function are explicitly set `to` `trade.sellToken` and `trade.buyToken` respectively. Thus, the swap is restricted `to` only `trade.sellToken` and `trade.buyToken`, which points `to` either the primary or secondary token of the pool. This prevents other tokens that reside in the vault `from` being swapped out.\\nHowever, this measure was not applied to the `EXACT_IN_BATCH` trade as it ignores the `trade.sellToken` and `trade.buyToken` , and allow the caller to define arbitrary `data.route` where the first route (_route[0]) and last route (_route[last_index]) could be any token.\\nThe vault will hold the reward tokens (CRV, CVX, LDO) when the vault administrator claims the rewards or a malicious user claims the rewards on behalf of the vault by calling Convex's getReward function.\\nAssume that attacker is faster than the admin calling the reinvest function. There is a possibility that an attacker executes a `EXACT_IN_BATCH` trade and specifies the `_route[0]` as one of the reward tokens residing on the vault and swaps away the reward tokens during depositing (_tradePrimaryForSecondary) or redemption (_sellSecondaryBalance). In addition, an attacker could also sell away the Convex deposit tokens if a new exploit is found.\\nIn addition, the vault also holds Convex deposit tokens, which represent assets held by the vault.\\nThis issue affects the in-scope `CurveV2Adapter` and `CurveAdapter` since they do not validate the `data.route` provided by the users.\\nCurveV2Adapter\\n```\\nFile: CurveV2Adapter.sol\\n    function getExecutionData(address from, Trade calldata trade)\\n        internal view returns (\\n            address spender,\\n            address target,\\n            uint256 msgValue,\\n            bytes memory executionCallData\\n        )\\n    {\\n        if (trade.tradeType == TradeType.EXACT_IN_SINGLE) {\\n            CurveV2SingleData memory data = abi.decode(trade.exchangeData, (CurveV2SingleData));\\n            executionCallData = abi.encodeWithSelector(\\n                ICurveRouterV2.exchange.selector,\\n                data.pool,\\n                _getTokenAddress(trade.sellToken),\\n                _getTokenAddress(trade.buyToken),\\n                trade.amount,\\n                trade.limit,\\n                address(this)\\n            );\\n        } else if (trade.tradeType == TradeType.EXACT_IN_BATCH) {\\n            CurveV2BatchData memory data = abi.decode(trade.exchangeData, (CurveV2BatchData));\\n            // Array of pools for swaps via zap contracts. This parameter is only needed for\\n            // Polygon meta-factories underlying swaps.\\n            address[4] memory pools;\\n            executionCallData = abi.encodeWithSelector(\\n                ICurveRouterV2.exchange_multiple.selector,\\n                data.route,\\n                data.swapParams,\\n                trade.amount,\\n                trade.limit,\\n                pools,\\n                address(this)\\n            );\\n```\\n\\nCurveAdapter\\n```\\nFile: CurveAdapter.sol\\n    function _exactInBatch(Trade memory trade) internal view returns (bytes memory executionCallData) {\\n        CurveBatchData memory data = abi.decode(trade.exchangeData, (CurveBatchData));\\n\\n        return abi.encodeWithSelector(\\n            ICurveRouter.exchange.selector,\\n            trade.amount,\\n            data.route,\\n            data.indices,\\n            trade.limit\\n        );\\n    }\\n```\\n\\nFollowing are some examples of where this vulnerability could potentially be exploited. Assume a vault that supports the CurveV2's ETH/stETH pool.\\nPerform the smallest possible redemption to trigger the `_sellSecondaryBalance` function. Configure the `RedeemParams` to swap the reward token (CRV, CVX, or LDO) or Convex Deposit token for the primary token (ETH). This will cause the `finalPrimaryBalance` to increase by the number of incoming primary tokens (ETH), thus inflating the number of primary tokens redeemed.\\nPerform the smallest possible deposit to trigger the `_tradePrimaryForSecondary`. Configure `DepositTradeParams` to swap the reward token (CRV, CVX, or LDO) or Convex Deposit token for the secondary tokens (stETH). This will cause the `secondaryAmount` to increase by the number of incoming secondary tokens (stETH), thus inflating the number of secondary tokens available for the deposit.\\nUpon further investigation, it was observed that the vault would only approve the exchange to pull the `trade.sellToken`, which points to either the primary token (ETH) or secondary token (stETH). Thus, the reward tokens (CRV, CVX, or LDO) or Convex deposit tokens cannot be sent to the exchanges. Thus, the vault will not be affected if none of the reward tokens/Convex Deposit tokens equals the primary or secondary token.\\n```\\nFile: TradingUtils.sol\\n    /// @notice Approve exchange to pull from this contract\\n    /// @dev approve up to trade.amount for EXACT_IN trades and up to trade.limit\\n    /// for EXACT_OUT trades\\n    function _approve(Trade memory trade, address spender) private {\\n        uint256 allowance = _isExactIn(trade) ? trade.amount : trade.limit;\\n        address sellToken = trade.sellToken;\\n        // approve WETH instead of ETH for ETH trades if\\n        // spender != address(0) (checked by the caller)\\n        if (sellToken == Constants.ETH_ADDRESS) {\\n            sellToken = address(Deployments.WETH);\\n        }\\n        IERC20(sellToken).checkApprove(spender, allowance);\\n    }\\n```\\n\\nHowever, there might be some Curve Pools or Convex's reward contracts whose reward tokens are similar to the primary or secondary tokens of the vault. If the vault supports those pools, the vault will be vulnerable. In addition, the reward tokens of a Curve pool or Convex's reward contracts are not immutable. It is possible for the governance to add a new reward token that might be the same as the primary or secondary token.
It is recommended to implement additional checks when performing a `EXACT_IN_BATCH` trade with the `CurveV2Adapter` or `CurveAdapter` adaptor. The first item in the route must be the `trade.sellToken`, and the last item in the route must be the `trade.buyToken`. This will restrict the `trade.sellToken` to the primary or secondary token, and prevent reward and Convex Deposit tokens from being sold (Assuming primary/secondary token != reward tokens).\\n```\\nroute[0] == trade.sellToken\\nroute[last index] == trade.buyToken\\n```\\n\\nThe vault holds many Convex Deposit tokens (e.g. cvxsteCRV). A risk analysis of the vault shows that the worst thing that could happen is that all the Convex Deposit tokens are swapped away if a new exploit is found, which would drain the entire vault. For defense-in-depth, it is recommended to check that the selling token is not a Convex Deposit token under any circumstance when using the trade adaptor.\\nThe trade adaptors are one of the attack vectors that the attacker could potentially use to move tokens out of the vault if any exploit is found. Thus, they should be locked down or restricted where possible.\\nAlternatively, consider removing the `EXACT_IN_BATCH` trade function from the affected adaptors to reduce the attack surface if the security risk of this feature outweighs the benefit of the batch function.
There is a risk that the reward tokens or Convex deposit tokens of the vault are sold by malicious users under certain conditions (e.g. if reward tokens are equal to primary/secondary tokens OR a new exploit is found in other parts of the code), thus potentially draining assets from the vault.
```\\nFile: CurveV2Adapter.sol\\n    function getExecutionData(address from, Trade calldata trade)\\n        internal view returns (\\n            address spender,\\n            address target,\\n            uint256 msgValue,\\n            bytes memory executionCallData\\n        )\\n    {\\n        if (trade.tradeType == TradeType.EXACT_IN_SINGLE) {\\n            CurveV2SingleData memory data = abi.decode(trade.exchangeData, (CurveV2SingleData));\\n            executionCallData = abi.encodeWithSelector(\\n                ICurveRouterV2.exchange.selector,\\n                data.pool,\\n                _getTokenAddress(trade.sellToken),\\n                _getTokenAddress(trade.buyToken),\\n                trade.amount,\\n                trade.limit,\\n                address(this)\\n            );\\n        } else if (trade.tradeType == TradeType.EXACT_IN_BATCH) {\\n            CurveV2BatchData memory data = abi.decode(trade.exchangeData, (CurveV2BatchData));\\n            // Array of pools for swaps via zap contracts. This parameter is only needed for\\n            // Polygon meta-factories underlying swaps.\\n            address[4] memory pools;\\n            executionCallData = abi.encodeWithSelector(\\n                ICurveRouterV2.exchange_multiple.selector,\\n                data.route,\\n                data.swapParams,\\n                trade.amount,\\n                trade.limit,\\n                pools,\\n                address(this)\\n            );\\n```\\n