Description

With the uniswapSimpleSwap function, users can use Uniswap to swap one asset for another, paying with a note in the darkpool and obtaining a new note for the swapped asset. The function takes a proof as well as the following struct UniswapSimpleSwapArgs as arguments:

struct UniswapSimpleSwapArgs {
    // data of the note that will be used for swap
    UniswapNoteData inNoteData;
    // merkle root of the merkle tree that the commitment of the note is included
    bytes32 merkleRoot;
    // address of the asset that will be received after swap
    address assetOut;
    // address of the relayer
    address payable relayer;
    // minimum amount of the asset that will be received after swap
    uint256 amountOutMin;
    // note footer of the note created after swap
    bytes32 noteFooter;
    // gas fee of the relayer
    uint256 relayerGasFee;
    // pool fee of the swap (Uniswap)
    uint24 poolFee;
}

struct UniswapNoteData {
    address assetAddress;
    uint256 amount;
    bytes32 nullifier;
}

The proof checks that a note with the nullifier inNoteDate.nullifier is present in the Merkle tree with root hash merkleRoot with asset inNoteDate.assetAddress and amount inNoteDate.amount. It is also checked that noteFooter belongs to the same user and that the user signed the input note and nullifier as well as the output-note footer and asset.

However, relayer, relayerGasFee, amountOutMin, and poolFee are not used in the proof, and in particular, are not checked to be signed by the user owning the input note.

As the uniswapSimpleSwap function can be called by anyone, an attacker getting knowledge of the arguments to a call to uniswapSimpleSwap (for example by observing a transaction including it in the mempool or by acting as a relayer for the call) can then front-run the call with their own transaction, where they can choose the above arguments freely.

The impact of this regarding manipulation of relayer and relayerGasFee is discussed in a separate finding; see Finding ref↗.

An attacker can submit the transaction with amountOutMin=0 and poolFee set to a value for which the Uniswap pool for the relevant asset pair has very low liquidity. By preceding the call to uniswapSimpleSwap with themselves adding the appropriate liquidity to the pool at the highest or lowest price possible, the swap carried out by uniswapSimpleSwap for the input note's owner will be done at an extremely unfavorable price. The attacker will thus have been able to buy the value of the input note (minus the service fee) for (next to) nothing, essentially stealing the input note's value.

Impact

Attackers front-running calls to uniswapSimpleSwap can steal the value of the input note.

Recommendations

Ensure that the user owning the input note approves of relayer, relayerGasFee, amountOutMin, and poolFee by including those values as public circuit variables in the uniswap_swap circuit and including them in the signature signed by the user's private key.

Remediation

This issue has been acknowledged by Singularity. In commit , amountOutMin and poolFee were added as public circuit variables in the uniswap_swap circuit and included in the signature signed by the user's private key. The analogous change for relayer was implemented in commit .

Singularity informed us that initially, only Singularity will run relayers, and that gas fees will be handled before third parties can run relayers.