Zellic - Audit Reports

Category: Business Logic

The RouteProcessor3 should not hold nontransient tokens

Informational Severity

Informational Impact

N/A Likelihood

Description

There are numerous ways in which tokens can be stolen if they are held by the RouteProcessor3.

For example, one way is by directly asking the contract to wrap or unwrap Ether and transfer it to the user.

function wrapNative(uint256 stream, address from, address tokenIn, uint256 amountIn) private {
    uint8 directionAndFake = stream.readUint8();
    address to = stream.readAddress();

    if (directionAndFake & 1 == 1) {  // wrap native
        address wrapToken = stream.readAddress();
        if (directionAndFake & 2 == 0) IWETH(wrapToken).deposit{value: amountIn}();
        if (to != address(this)) IERC20(wrapToken).safeTransfer(to, amountIn);
    } else { // unwrap native
        if (directionAndFake & 2 == 0) {
            if (from != address(this)) IERC20(tokenIn).safeTransferFrom(from, address(this), amountIn);
            IWETH(tokenIn).withdraw(amountIn);
        }
        payable(to).transfer(address(this).balance);
    }
}

The wrapNative function can be reached with from == address(this) by going from processRouteInternal to processNative, listed below,

function processNative(uint256 stream) private {
    uint256 amountTotal = address(this).balance;
    distributeAndSwap(stream, address(this), NATIVE_ADDRESS, amountTotal);
}

then requesting a wrapNative operation in the swap. The tokens belonging to RouteProcessor3 will then be wrapped or unwrapped and transferred to the user.

Impact

The RouteProcessor3 contract should not hold tokens except transiently, in the middle of a transaction.

Recommendations

Document prominently that the RouteProcessor3 contract should not hold tokens.

Remediation

This issue has been acknowledged by Sushiswap.