Malicious 0x Order Can Extract Value From The Vault During Trade

Summary

A malicious user can craft a malicious 0x order when settling the vault or reinvesting rewards to extract value from the vault by maximizing the allowed slippage, leading to loss of assets for the vault and its shareholders.

Vulnerability Detail

Per 0x documentation (https://protocol.0x.org/en/latest/basics/orders.html#orders and https://docs.0x.org/), users are allowed to specify the taker of the order. If the taker is configured, only the address configured in the taker fill can fulfill the order. Following is the description of the taker field taken from the documentation:

taker - The address of the party that is allowed to fill the order. If set to a specific party, the order cannot be filled by anyone else. If left unspecified, anyone can fill the order.

Based on the current implementation of the 0x adaptor, it was observed that the 0x adaptor did not ensure that taker field of a 0x order is set to zero. As such, it is possible for a malicious user to perform the trade with a 0x order that can only be fulfilled by himself.

The settleVault and reinvestReward functions of a vault are permissionless and can be called by anyone. When settling a vault or reinvesting the rewards in a vault, a large number of assets will be traded in the open market. The caller has the flexibility to define how the trade should be executed. A malicious user can trigger these functions and configure the trade to be executed via 0x DEX and craft a malicious 0x order with the taker field set to himself. The malicious user will have the sole right to fulfill the order and will be able to extract value from the trade by maximizing the allowed slippage.

The following shows that there is no validation against the taker field of an 0x order within the 0x adaptor contract.

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: }

Impact

Loss of assets for the vault and its vault shareholders. This is because the assets are traded at maximum slippage by malicious users to extract value from the trade. Trading is a zero-sum game, and therefore, the attacker's gain is the vault and its shareholders' loss.

Since the number of assets traded will be significant, especially during vault settlement of CrossCurrencyfCashVault USDC/DAI vault where all the DAI tokens belonging to all vault shareholders within the vault will be traded to USDC, the amount of slippage will be amplified making this extremely profitable and attractive for malicious users. Most vaults allow 10x and 20x leverage, so the amount of DAI in the vaults to be traded is huge. Thus, the impact of this issue will be significant.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider implementing additional validation to ensure that the taker field of a 0x order is set to address(0) so that anyone can fill the order in the market and prevent malicious users from restricting the trade to themselves.

require(taker == address(0), "0x order cannot set taker");

Reference

A similar issue was found in the past audit report:

https://code4rena.com/reports/2022-01-notional/#m-07-_validateorder-does-not-allow-anyone-to-be-a-taker-of-an-off-chain-order

Duplicate of #75

sherlock-audit / 2022-09-notional-judging

xiaoming90 - Malicious 0x Order Can Extract Value From The Vault During Trade #86