ETH can be sold during reinvestment
The existing control to prevent ETH from being sold during reinvestment can be bypassed, allowing the bots to accidentally or maliciously sell off the non-reward assets of the vault.\\nMultiple instances of this issue were found:\\nInstance 1 - Curve's Implementation\\nThe `_isInvalidRewardToken` function attempts to prevent the callers from selling away ETH during reinvestment.\\n```\\nFile: ConvexStakingMixin.sol\\n    function _isInvalidRewardToken(address token) internal override view returns (bool) {\\n        return (\\n            token == TOKEN_1 ||\\n            token == TOKEN_2 ||\\n            token == address(CURVE_POOL_TOKEN) ||\\n            token == address(CONVEX_REWARD_POOL) ||\\n            token == address(CONVEX_BOOSTER) ||\\n            token == Deployments.ALT_ETH_ADDRESS\\n        );\\n    }\\n```\\n\\nHowever, the code at Line 67 above will not achieve the intended outcome as `Deployments.ALT_ETH_ADDRESS` is not a valid token address in the first place.\\n```\\naddress internal constant ALT_ETH_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;\\n```\\n\\nWhen the caller is executing a trade with ETH, the address for ETH used is either `Deployments.WETH` or `Deployments.ETH_ADDRESS` (address(0)) as shown in the TradingUtils's source code, not the `Deployments.ALT_ETH_ADDRESS`.\\n```\\nFile: TradingUtils.sol\\n    function _executeTrade(\\n        address target,\\n        uint256 msgValue,\\n        bytes memory params,\\n        address spender,\\n        Trade memory trade\\n    ) private {\\n        uint256 preTradeBalance;\\n \\n        if (trade.buyToken == address(Deployments.WETH)) {\\n            preTradeBalance = address(this).balance;\\n        } else if (trade.buyToken == Deployments.ETH_ADDRESS || _needsToUnwrapExcessWETH(trade, spender)) {\\n            preTradeBalance = IERC20(address(Deployments.WETH)).balanceOf(address(this));\\n        }\\n```\\n\\nAs a result, the caller (bot) of the reinvestment function could still sell off the ETH from the vault, bypassing the requirement.\\nInstance 2 - Balancer's Implementation\\nWhen the caller is executing a trade with ETH, the address for ETH used is either `Deployments.WETH` or `Deployments.ETH_ADDRESS` (address(0)), as mentioned earlier. However, the `AuraStakingMixin._isInvalidRewardToken` function only blocks `Deployments.WETH` but not `Deployments.ETH`, thus allowing the caller (bot) of the reinvestment function, could still sell off the ETH from the vault, bypassing the requirement.\\n```\\nFile: AuraStakingMixin.sol\\n    function _isInvalidRewardToken(address token) internal override view returns (bool) {\\n        return (\\n            token == TOKEN_1 ||\\n            token == TOKEN_2 ||\\n            token == TOKEN_3 ||\\n            token == TOKEN_4 ||\\n            token == TOKEN_5 ||\\n            token == address(AURA_BOOSTER) ||\\n            token == address(AURA_REWARD_POOL) ||\\n            token == address(Deployments.WETH)\\n        );\\n    }\\n```\\n\\nPer the sponsor's clarification below, the contracts should protect against the bot doing unintended things (including acting maliciously) due to coding errors, which is one of the main reasons for having the `_isInvalidRewardToken` function. Thus, this issue is a valid bug in the context of this audit contest.\\n
To ensure that ETH cannot be sold off during reinvestment, consider the following changes:\\nCurve\\n```\\nFile: ConvexStakingMixin.sol\\nfunction _isInvalidRewardToken(address token) internal override view returns (bool) {\\n    return (\\n        token == TOKEN_1 ||\\n        token == TOKEN_2 ||\\n        token == address(CURVE_POOL_TOKEN) ||\\n        token == address(CONVEX_REWARD_POOL) ||\\n        token == address(CONVEX_BOOSTER) ||\\n// Add the line below\\n       token == Deployments.ETH ||\\n// Add the line below\\n       token == Deployments.WETH ||\\n        token == Deployments.ALT_ETH_ADDRESS\\n    );\\n}\\n```\\n\\nBalancer\\n```\\nFile: AuraStakingMixin.sol\\nfunction _isInvalidRewardToken(address token) internal override view returns (bool) {\\n    return (\\n        token == TOKEN_1 ||\\n        token == TOKEN_2 ||\\n        token == TOKEN_3 ||\\n        token == TOKEN_4 ||\\n        token == TOKEN_5 ||\\n        token == address(AURA_BOOSTER) ||\\n        token == address(AURA_REWARD_POOL) ||\\n// Add the line below\\n  token == address(Deployments.ETH)        \\n        token == address(Deployments.WETH)\\n    );\\n}\\n```\\n
The existing control to prevent ETH from being sold during reinvestment can be bypassed, allowing the bots to accidentally or maliciously sell off the non-reward assets of the vault.
```\\nFile: ConvexStakingMixin.sol\\n    function _isInvalidRewardToken(address token) internal override view returns (bool) {\\n        return (\\n            token == TOKEN_1 ||\\n            token == TOKEN_2 ||\\n            token == address(CURVE_POOL_TOKEN) ||\\n            token == address(CONVEX_REWARD_POOL) ||\\n            token == address(CONVEX_BOOSTER) ||\\n            token == Deployments.ALT_ETH_ADDRESS\\n        );\\n    }\\n```\\n