Zellic - Audit Reports

Category: Business Logic

Same token swap is allowed

Low Severity

Low Impact

Medium Likelihood

Description

A user might mistakenly perform a same-token swap via the protocol, since there are no restrictions against that.

Impact

In function _swap() there are no checks whatsoever for whether the tokens[0] and tokens[1] are identical.

function _swap(
    address custodian,
    address[] calldata tokens,
    uint256[] calldata amounts,
    uint64 deadline,
    bytes calldata signature
) internal onlyWhitelisted(custodian) returns (bool) {
    Swap memory swap = Swap({
        user: msg.sender,
        custodian: custodian,
        token0: tokens[0],
        token1: tokens[1],
        amount0: amounts[0],
        amount1: amounts[1],
        deadline: deadline,
        nonce: nonces[msg.sender],
        chainId: chainId
    });

    require(block.timestamp < swap.deadline, "Expired Order");
    require(verify(swap, signature), "Invalid Signer");
    require(swap.amount1 > 0 && swap.amount0 > 0, "amount != 0");

This can lead to loss of the gas cost used in the transaction, as well as the tokens lost to protocol fees, all due to an undesireable action performed by the user in the first place.

Recommendations

We recommend adding an additional check when performing a swap, such that the tokens on either side of the swap are not the same.

function _swap(
    address custodian,
    address[] calldata tokens,
    uint256[] calldata amounts,
    uint64 deadline,
    bytes calldata signature
) internal onlyWhitelisted(custodian) returns (bool) {
    require(tokens[0] != tokens[1], "Same token swap is disallowed");
    Swap memory swap = Swap({
        user: msg.sender,
        custodian: custodian,
        token0: tokens[0],
        token1: tokens[1],
        amount0: amounts[0],
        amount1: amounts[1],
        deadline: deadline,
        nonce: nonces[msg.sender],
        chainId: chainId
    });

    require(block.timestamp < swap.deadline, "Expired Order");
    require(verify(swap, signature), "Invalid Signer");
    require(swap.amount1 > 0 && swap.amount0 > 0, "amount != 0");

Remediation

This issue has been acknowledged by the Router team and mitigated in commit 3be1183↗.