Description

The deposit-limit feature limits the rate at which ERC-20 tokens can enter and be mixed by Nocturne. Another feature, the screener-signature requirement, allows Nocturne to identify the owner of funds entering the protocol in order to ensure legal compliance.

One way that funds can enter Nocturne without being subject to the deposit limit or the screener-signature requirement is in the form of refund notes. At any time, the owner of previously deposited notes can issue an operation that unwraps the notes, does a series of actions (whitelisted external calls) with them, and then any assets resulting from those calls are rewrapped into new notes. The example use case for this feature is to enable Uniswap swaps of hidden assets.

Nocturne takes many steps to prevent these actions from introducing new value into the protocol or introducing any reentrancy hazards, including ensuring that the tracked assets have zero balances before running the actions, requiring the top-level bundle submission call to be made from an EOA, and strictly whitelisting the calls an action can be. In the version of the configuration we audited, the only swap methods whitelisted were Uniswap's swapExactInput and swapExactInputSingle.

However, a check that was missed is the tokens specified in the Uniswap swap path, including the tokenIn and path parameters. If the tokenIn parameter or any token in the path parameter is an attacker-deployed ERC-20, Uniswap will call ERC20.transfer on that token, which means the attacker can execute arbitrary code in the middle of the action.

Impact

An attacker can cause arbitrary calls to be done in the middle of an action through an attacker-deployed ERC-20 token's ERC20.transfer function called by Uniswap during a swap.

These arbitrary calls can transfer funds into the Handler, which bypasses deposit limits and screener checks; reenter Nocturne functions not gated by a reentrancy guard; and execute attacks on other protocols in order to immediately deposit the proceeds from such exploitation into Nocturne.

Recommendations

The Handler must ensure that all tokens Uniswap calls transfer on are legitimate tokens, tokens that do not cause attacker-specified behavior when called.

For exactInputSingle, this means checking the tokenIn and tokenOut parameters, and for exactInput, this means deserializing the path parameter and checking each token in it.

This issue is difficult to remediate because many tokens would need to be whitelisted for the purpose of being on a Uniswap path. (This could be a separate, more lax whitelist than the whitelist of tokens that Nocturne is willing to store.) If the best execution price for a swap that a nonmalicious user wishes to execute has a path that contains a token that is not on the whitelist, that user will have to get a suboptimal execution price for the swap.

Remediation

This issue has been acknowledged by Nocturne, and a fix was implemented in commits and .