Description

For many transactions, relayers are sent a fee for their gas costs and other costs. As an example, let us consider the function withdrawERC20 of the DarkpoolAssetManager, which is implemented as follows:

function withdrawERC20(
        address _asset,
        bytes calldata _proof,
        bytes32 _merkleRoot,
        bytes32 _nullifier,
        address _recipient,
        address _relayer,
        uint256 _amount,
        uint256 _relayerGasFee
    ) public {
        require(
            _complianceManager.isPermitted(address(this), _recipient),
            "BaseAssetManager: invalid credential"
        );
        if (!_relayerHub.isRelayerRegistered(_relayer)) {
            revert RelayerNotRegistered();
        }
        if (!_merkleTreeOperator.nullifierIsNotUsed(_nullifier)) {
            revert NullifierUsed();
        }
        if (!_merkleTreeOperator.merkleRootIsAllowed(_merkleRoot)) {
            revert MerkleRootNotAllowed();
        }
        WithdrawRawInputs memory inputs = WithdrawRawInputs(
            _recipient,
            _merkleRoot,
            _asset,
            _amount,    
            _nullifier
        );

        _verifyProof(_proof, _buildWithdrawInputs(inputs), "withdraw");

        _releaseERC20WithFee(
            _asset,
            _recipient,
            _relayer,
            _relayerGasFee,
            _amount
        );

        _postWithdraw(_nullifier);

        emit Withdraw(_nullifier, _amount, _asset, _recipient);
    }

Note that this function is public and msg.sender is not checked against anything, so this function is callable by anyone. We focus on the two arguments _relayer and _relayerGasFee. They only occur in the following two calls:

if (!_relayerHub.isRelayerRegistered(_relayer)) {
    revert RelayerNotRegistered();
}

_releaseERC20WithFee(
    _asset,
    _recipient,
    _relayer,
    _relayerGasFee,
    _amount
);

The first call checks that _relayer is a registered relayer. Relayers can be registered by the owner of the relayer hub.

The second call will transfer a certain service fee to the fee manager, transfer an amount of _relayerGasFee of asset to relayer, and transfer the rest to _recipient.

As long as the service fee serviceFee plus _relayerGasFee is less than _amount, this will not revert. The argument _relayerGasFee can thus be freely chosen by the caller as long as it satisfies _relayerGasFee < _amount - serviceFee. Similarly, _relayer can be freely chosen as long as the address is registered as a relayer.

Relayers are registered by the owner of the relayer hub. Logic deciding who will get registered is not in scope for this audit. Singularity informed us that the plan is to decentralize relayers, with anyone staking certain tokens having the possibility to become relayers. With that context, we judge that relayers cannot be considered trusted, though the staking aspect may introduce certain hurdles for potential attackers. Because of this we rate likelihood as Medium.

Impact

A user may expect a certain relayer A to submit the transaction for a certain small fee. Relayer A could instead submit the transaction with _relayerGasFee set to _amount - serviceFee - 1, pocketing (nearly) the entire withdrawal (minus the service fee), leaving the user with (next to) nothing.

Another relayer B might also see the transaction including a call to, for example, withdrawERC20 in the mempool and front-run it with a call to withdrawERC20, in which they replace _relayer with their own address and _relayerGasFee with _amount - serviceFee - 1.

The following entry points are vulnerable to this issue:

• DarkpoolAssetManager's withdrawETH and withdrawERC20

• UniswapSwapAssetManager's uniswapSimpleSwap

• UniswapLiquidityAssetManager's uniswapLiquidityProvision, uniswapCollectFees, and uniswapRemoveLiquidity

• CurveAddLiquidityAssetManager's curveAddLiquidity

• CurveFSNAddLiquidityAssetManager's curveAddLiquidity

• CurveFSNRemoveLiquidityAssetManager's curveRemoveLiquidity

• CurveMPAddLiquidityAssetManager's curveAddLiquidity

• CurveMPRemoveLiquidityAssetManager's curveRemoveLiquidity

• CurveMultiExchangeAssetManager's curveMultiExchange

• CurveRemoveLiquidityAssetManager's curveRemoveLiquidity

• CurveSingleExchangeAssetManager's curveSingleExchange

Recommendations

Ensure that the user owning the funds from which the fee is paid approves of the relayer and relayer fee by including those two values as public circuit variables in the relevant circuit and including them in the signature signed by the user's private key.

Remediation

This issue has been acknowledged by Singularity. In commit , relayer was added as public circuit variables in the relevant circuits and included in the signature signed by the user's private key.

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