mstpr-brainbot - If the users requested `toToken` is ETH then the users funds will be stuck in the bridge

[sherlock-admin4]

mstpr-brainbot

high

If the users requested toToken is ETH then the users funds will be stuck in the bridge

Summary

Users can initiate a cross chain swap from one chain to other as long as both chains are allowed. Users can select the destination token to any token, including native ETH. If the chosen token is ETH then the bridging will revert, hence the users funds will be stuck.

Vulnerability Detail

Let's assume an user comes and initiates a bridge from chainX to chainY with fromToken WETH and destination chain toToken as ETH:

When the LayerZero endpoint calls the StargateBridge in destination chain, the bridge will call the StargateRouter's swapRemote function in destination chain.

When we come to these lines in below code snippet in the swapRemote execution flow, the pool id will be fetched and the pool id's underlying token will be the SGETH token in that chain. (Example: https://etherscan.io/address/0x101816545F6bd2b1076434B54383a1E633390A2E#readContract SGETH pool in mainnet, check the "token" variable, it is the SGETH token, not ETH or WETH)

Then, the pools swapRemote function will be called which as we can see in the SGETH pool implementation, the "to" address will receive the SGETH tokens. Which the "to" address is the WooCrosschainRouter contract that will be receiving SGETH tokens.

Then, the IStargateReceiver interface sgReceive function will be called in the WooCrosschainRouter contract and as we can observe the 4th argument, which corresponds to bridgedToken in WooCrosschainRouter implementation, will be pool.token(), which is SGETH.

-> Pool pool = _getPool(_dstPoolId);
        // first try catch the swap remote
        -> try pool.swapRemote(_srcChainId, _srcPoolId, _to, _s) returns (uint256 amountLD) {
            if (_payload.length > 0) {
                // then try catch the external contract call
                -> try IStargateReceiver(_to).sgReceive{gas: _dstGasForCall}(_srcChainId, _srcAddress, _nonce, pool.token(), amountLD, _payload) {
                    // do nothing
                } catch (bytes memory reason) {
                    cachedSwapLookup[_srcChainId][_srcAddress][_nonce] = CachedSwap(pool.token(), amountLD, _to, _payload);
                    emit CachedSwapSaved(_srcChainId, _srcAddress, _nonce, pool.token(), amountLD, _to, _payload, reason);
                }
            }
        } catch {
             .
             .

When the sgReceive function called, since the bridgedToken is SGETH the first "if" statement will be executed, which is an internal call to _handleNativeReceived(). Also, the decoded payloads toToken variable will be the ETH placeholder address since the user sent the request as the bridge destination token as ETH.

function sgReceive(
        uint16, // srcChainId
        bytes memory, // srcAddress
        uint256, // nonce
        address bridgedToken,
        uint256 amountLD,
        bytes memory payload
    ) external {
        require(msg.sender == sgInfo.sgRouter(), "WooCrossChainRouterV3: INVALID_CALLER");

        // make sure the same order to abi.encode when decode payload
        (uint256 refId, address to, address toToken, uint256 minToAmount, Dst1inch memory dst1inch) = abi.decode(
            payload,
            (uint256, address, address, uint256, Dst1inch)
        );

        // toToken won't be SGETH, and bridgedToken won't be ETH_PLACEHOLDER_ADDR
        if (bridgedToken == sgInfo.sgETHs(sgInfo.sgChainIdLocal())) {
            // bridgedToken is SGETH, received native token
            -> _handleNativeReceived(refId, to, toToken, amountLD, minToAmount, dst1inch);
        } else {
            .
            .
    }

So far the WooCrosschainRouter has SGETH tokens in its balance and user requested ETH. Since the toToken == ETH_PLACEHOLDER_ADDR the first "if" statement will be executed. However, this is the part where the tx will revert because the function will try to send ETH from the WooCrosschainRouter to user but the router does not have ETH balance yet. Router has SGETH tokens.

function _handleNativeReceived(
        uint256 refId,
        address to,
        address toToken,
        uint256 bridgedAmount,
        uint256 minToAmount,
        Dst1inch memory dst1inch
    ) internal {
        address msgSender = _msgSender();

        -> if (toToken == ETH_PLACEHOLDER_ADDR) {
            // Directly transfer ETH
            -> TransferHelper.safeTransferETH(to, bridgedAmount);
            emit WooCrossSwapOnDstChain(
                refId,
                msgSender,
                to,
                weth,
                bridgedAmount,
                toToken,
                ETH_PLACEHOLDER_ADDR,
                minToAmount,
                bridgedAmount,
                dst1inch.swapRouter == address(0) ? 0 : 1,
                0
            );
            return;
        }
       .
       .

Impact

Users bridged funds in source chain will be stuck and will not receive any tokens in exchange in destination chain. Loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/CrossChain/WooCrossChainRouterV4.sol#L66-L181

https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/CrossChain/WooCrossChainRouterV4.sol#L269-L383

Tool used

Manual Review

Recommendation

[0xmer1in]

Can you provide the PoC? I think you might miss the SGETH transfer code, and I attach the code snippet here:

    function transfer(address dst, uint wad) external override returns (bool) {
        return transferFrom(msg.sender, dst, wad);
    }

    function transferFrom(address src, address dst, uint wad) public override nonReentrant returns (bool) {
        require(balanceOf[src] >= wad);

        if (src != msg.sender && allowance[src][msg.sender] != uint(-1)) {
            require(allowance[src][msg.sender] >= wad);
            allowance[src][msg.sender] -= wad;
        }

        // always decrement the src (payer) address
        balanceOf[src] -= wad;

        if(noUnwrapTo[dst]){
            // we do *not* unwrap
            balanceOf[dst] += wad;
            emit Transfer(src, dst, wad);

        } else {
            // unwrap and send native gas token
            totalSupply -= wad; // if its getting unwrapped, decrement the totalSupply
            (bool success, ) = dst.call{value: wad}("");
            require(success, "SGETH: failed to transfer");
            emit TransferNative(src, dst, wad);
        }

        return true;
    }

pool's swapRemote being called by Router, at that time, _to is StargateComposer, which will not unwrap because noUnwrapTo[dst] is true, then the Router call the StargateComposer's sgReceive.

In StargateComposer's sgReceive, it call SGETH's transfer with dst(which is to) is WooCrossChainRouterV4, according to noUnwrapTo[dst] is false, the wad SGETH will be unwrapped and transfer the wad native ETH to WooCrossChainRouterV4.

Example txn in Optimism(you can see the Transfer event emits first and TransferNative event emits later then by the SGETH contract address): https://optimistic.etherscan.io/tx/0xd94786a40ff4864704d9122437214952a7258e9c704acc24015d0801d68fcc1d

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

sorry but I don't see where we call stargate's swapRemote; in woffi's crossSwap we call bridgeByStargate and it calls stargates swap not swapRemote and I don't where we use swapRemote; therefore; I assume it's invalid; the report doesn't say it either; watson just said user wants to initiates the swap and then whe LZ calls stargates swapRemote but I don't see it gets called

[WangSecurity]

request poc

[sherlock-admin3]

PoC requested from @mstpr

Requests remaining: 7