Bought/Purchased Token Can Be Sent To Attacker's Wallet Using 0x Adaptor

Summary

The lack of recipient validation against the 0x order within the 0x adaptor (ZeroExAdapter) allows the purchased/output tokens of the trade to be sent to the attacker's wallet.

Vulnerability Detail

Background

How does the emergency vault settlement process work?

Anyone can call the settleVaultEmergency function to trigger the emergency vault settlement as it is permissionless
The _getEmergencySettlementParams function will calculate the excess BPT tokens within the vault to be settled/sold
The amount of excess BPT tokens will be converted to an equivalence amount of strategy tokens to be settled
The strategy tokens will be settled by withdrawing staked BPT tokens from Aura Finance back to the vault for redemption.
The vault will then redeem the BTP tokens from Balancer to redeem its underlying assets (WETH and stETH)
The primary and secondary assets of the vault are WETH and stETH respectively. The secondary asset (stETH) will be traded for the primary asset (WETH) in one of the supported DEXes. In the end, only the primary assets (WETH) should remain within the vault.
The WETH within the vault will be sent to Notional, and Notional will mint the asset tokens (cEther) for the vault in return.
After completing the emergency vault settlement process, the vault will gain asset tokens (cEther) after settling/selling its excess BPT tokens.

Issue Description

The caller of the settleVaultEmergency function can specify the trade parameters to sell the secondary tokens (stETH) for primary tokens (WETH) in any of the supported 5 DEX protocols (Curve, Balancer V2, Uniswap V2 & V3 and 0x) in Step 5 of the above emergency vault settlement process.

After analyzing the adaptors of 5 DEX protocols (Curve, Balancer V2, Uniswap V2 & V3 and 0x), it was observed that Curve, Balancer V2, Uniswap V2, and Uniswap V3 are designed in a way that the purchased tokens can only be returned to the vault.

Take the Uniswap V2 adaptor as an example. When the vault triggers the trade execution, it will always pass its own address address(this) to the from parameter of the getExecutionData function. The value of from parameter will be passed to the to parameter of Uniswap's swapExactTokensForTokens function, which indicates the recipient of the output/purchased tokens. Therefore, it is impossible for the caller to specify the recipient of the output tokens to another address. This is also the same for Curve, Balancer V2, and Uniswap V3.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/UniV2Adapter.sol#L12

File: UniV2Adapter.sol
12:     function getExecutionData(address from, Trade calldata trade)
..SNIP..
31:             executionCallData = abi.encodeWithSelector(
32:                 IUniV2Router2.swapExactTokensForTokens.selector,
33:                 trade.amount,
34:                 trade.limit,
35:                 data.path,
36:                 from,
37:                 trade.deadline
38:             );

Note: Specification of swapExactTokensForTokens function can be found at https://docs.uniswap.org/protocol/V2/reference/smart-contracts/router-02#swapexacttokensfortokens

However, this is not implemented for the 0x adaptor (ZeroExAdapter). The from of the getExecutionData is completely ignored, and the caller has the full flexibility of crafting an order that benefits the caller.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/ZeroExAdapter.sol#L7

File: ZeroExAdapter.sol
07: library ZeroExAdapter {
08:     /// @dev executeTrade validates pre and post trade balances and also
09:     /// sets and revokes all approvals. We are also only calling a trusted
10:     /// zero ex proxy in this case. Therefore no order validation is done
11:     /// to allow for flexibility.
12:     function getExecutionData(address from, Trade calldata trade)
13:         internal view returns (
14:             address spender,
15:             address target,
16:             uint256 /* msgValue */,
17:             bytes memory executionCallData
18:         )
19:     {
20:         spender = Deployments.ZERO_EX;
21:         target = Deployments.ZERO_EX;
22:         // msgValue is always zero
23:         executionCallData = trade.exchangeData;
24:     }
25: }

A number of features are supported by 0x. The full list of the supported features can be found here. Specifically, the following are the functions of attacker interest because it allows the attacker to configure the recipient parameter so that the bought tokens will be redirected to the attacker's wallet instead of the vault.

LiquidityProviderFeature - sellToLiquidityProvider

    /// @dev Sells `sellAmount` of `inputToken` to the liquidity provider
    ///      at the given `provider` address.
    /// @param inputToken The token being sold.
    /// @param outputToken The token being bought.
    /// @param provider The address of the on-chain liquidity provider
    ///        to trade with.
    /// @param recipient The recipient of the bought tokens. If equal to
    ///        address(0), `msg.sender` is assumed to be the recipient.
    /// @param sellAmount The amount of `inputToken` to sell.
    /// @param minBuyAmount The minimum acceptable amount of `outputToken` to
    ///        buy. Reverts if this amount is not satisfied.
    /// @param auxiliaryData Auxiliary data supplied to the `provider` contract.
    /// @return boughtAmount The amount of `outputToken` bought.
    function sellToLiquidityProvider(
        IERC20TokenV06 inputToken,
        IERC20TokenV06 outputToken,
        ILiquidityProvider provider,
        address recipient,
        uint256 sellAmount,
        uint256 minBuyAmount,
        bytes calldata auxiliaryData
    )

UniswapV3Feature - sellTokenForTokenToUniswapV3

    /// @dev Sell a token for another token directly against uniswap v3.
    /// @param encodedPath Uniswap-encoded path.
    /// @param sellAmount amount of the first token in the path to sell.
    /// @param minBuyAmount Minimum amount of the last token in the path to buy.
    /// @param recipient The recipient of the bought tokens. Can be zero for sender.
    /// @return buyAmount Amount of the last token in the path bought.
    function sellTokenForTokenToUniswapV3(
        bytes memory encodedPath,
        uint256 sellAmount,
        uint256 minBuyAmount,
        address recipient
    )

The malicious user could perform the following actions to steal the assets:

Allow malicious users to specify the recipient of the output/purchased tokens to be themselves instead of the vault. This will cause the output/purchased tokens of the trade to be redirected to the malicious users instead of the vault
Specify the minBuyAmount parameter of the order to 1 WEI so that he only needs to provide 1 WEI to fill the order to obtain all the secondary token (stETH) that need to be sold. This is allowed as there is no slippage control within 0x adaptor (Refer to my "No Slippage Control If The Trade Executes Via 0x DEX During Emergency Vault Settlement" issue write-up)

Impact

Attackers can craft a 0x order that redirects the assets to their wallet, leading to loss of assets for the vaults and their users.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/UniV2Adapter.sol#L12 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/adapters/ZeroExAdapter.sol#L7

Tool used

Manual Review

Recommendation

It is recommended to implement validation against the submitted 0x trade order to ensure that the recipient of the bought tokens is set to the vault when using the 0x DEX. Consider implementing the following validation checks.

library ZeroExAdapter {
    /// @dev executeTrade validates pre and post trade balances and also
    /// sets and revokes all approvals. We are also only calling a trusted
    /// zero ex proxy in this case. Therefore no order validation is done
    /// to allow for flexibility.
    function getExecutionData(address from, Trade calldata trade)
        internal view returns (
            address spender,
            address target,
            uint256 /* msgValue */,
            bytes memory executionCallData
        )
    {
        spender = Deployments.ZERO_EX;
        target = Deployments.ZERO_EX;

        _validateExchangeData(from, trade);

        // msgValue is always zero
        executionCallData = trade.exchangeData;
    }

    function _validateExchangeData(address from, Trade calldata trade) internal pure {
        bytes calldata _data = trade.exchangeData;

        address inputToken;
        address outputToken;
        address recipient;
        uint256 inputTokenAmount;
        uint256 minOutputTokenAmount;

        require(_data.length >= 4, "Invalid calldata");
        bytes4 selector;
        assembly {
            selector := and(
                // Read the first 4 bytes of the _data array from calldata.
                calldataload(add(36, calldataload(164))), // 164 = 5 * 32 + 4
                0xffffffff00000000000000000000000000000000000000000000000000000000
            )
        }

        if (selector == 0xf7fcd384) {

            (
                inputToken, 
                outputToken, 
                , 
                recipient, 
                inputTokenAmount, 
                minOutputTokenAmount
            ) = abi.decode(_data[4:], (address, address, address, address, uint256, uint256));
            require(recipient == from, "Mismatched recipient");
        } else if (selector == 0x6af479b2) {
            // sellTokenForTokenToUniswapV3()
            bytes memory encodedPath;
            // prettier-ignore
            (
                encodedPath,
                inputTokenAmount, 
                minOutputTokenAmount, 
                recipient
            ) = abi.decode(_data[4:], (bytes, uint256, uint256, address));
            require(recipient == from, "Mismatched recipient");
        }
    }
}

sherlock-audit / 2022-09-notional-judging

xiaoming90 - Bought/Purchased Token Can Be Sent To Attacker's Wallet Using 0x Adaptor #75