0xadrii - Wrong usage of Stargate’s ETH router in balancer enables attackers to steal all bridged native funds

[sherlock-admin4]

0xadrii

high

Wrong usage of Stargate’s ETH router in balancer enables attackers to steal all bridged native funds

Summary

Using Stargate's ETH router will break rebalancing interactions when bridging native assets, making it possible for attackers to steal the native bridged funds.

Vulnerability Detail

Tapioca employs a rebalancer contract that constantly reweights the backing of mtOFT's across the mtOFT's supported networks using Stargate. When a rebalance() is triggered, the mtOFT underlying token will be router via Stargate to the destination chain mtOFT.

This vulnerability describes an issue that arises when bridging native tokens via Stargate’s ETH router.

Currently, underlying assets wrapped via mtOFTs are not stored in the mtOFT contract itself. Instead, they are deposited into an external TOFTVault contract. When assets are wrapped, they are transferred directly to the vault. Similarly, in order to unwrap assets they first need to be retrieved from the vault:

// BaseTOFT.sol

function _wrap(address _fromAddress, address _toAddress, uint256 _amount, uint256 _feeAmount) internal virtual {
        ...
        // IERC20(erc20).safeTransferFrom(_fromAddress, address(vault), _amount);
        bool isErr = pearlmit.transferFromERC20(_fromAddress, address(vault), erc20, _amount); 
        if (isErr) revert TOFT_NotValid();
        _mint(_toAddress, _amount - _feeAmount);
    } 

    function _wrapNative(address _toAddress, uint256 _amount, uint256 _feeAmount) internal virtual {
        vault.depositNative{value: _amount}();
        _mint(_toAddress, _amount - _feeAmount);
    }

    function _unwrap(address _toAddress, uint256 _amount) internal virtual {
        _burn(msg.sender, _amount);
        vault.withdraw(_toAddress, _amount);
    }

When a rebalance takes place in Balancer.sol, the following steps take place:

1. The assets to be bridged are extracted from the vault via ITOFT(_srcOft).extractUnderlying(_amount);
2. Assets are swapped and bridged using Stargate. An important concept to highlight here is that the router will bridge assets and send them to the destination chain’s mtOFT contract, not directly to the vault.

3. Because the second step does not directly swap and transfer assets into the vault, a last step is required in the destination chain so that the bridged assets can be actually sent to the vault. This is done via sgReceive(), which is a function that should be automatically called by Stargate’s router during the swap flow:

   // mtOFT.sol
   
   function sgReceive(uint16, bytes memory, uint256, address, uint256 amountLD, bytes memory) external payable {
           if (msg.sender != _stargateRouter) revert mTOFT_NotAuthorized();
   
           if (erc20 == address(0)) {
               vault.depositNative{value: amountLD}();
           } else {
               IERC20(erc20).safeTransfer(address(vault), amountLD);
           }
       }

The problem with the current implementation is that Stargate’s ETH router will NOT call sgReceive() during a swap call. This is because Stargate’s ETH router always assumes that the receiver in the destination chain is an EOA.

Proof of concept

Let’s examine how a Stargate swap flow works:

1. Tapioca's Balancer contract calls the router/ETH router’s swap()/swapETH() function. If swapETH() is called, the ETH router contract will internally call the regular router’s swap() function. It is crucial to note how the last field when calling swap() inside swapETH() is left with empty bytes (””), and a comment is left that reads: “empty payload, since sending to an EOA”.

   // Stargate's RouterETH.sol
   
   // compose stargate to swap ETH on the source to ETH on the destination
       function swapETH(
           uint16 _dstChainId,                         // destination Stargate chainId
           address payable _refundAddress,             // refund additional messageFee to this address
           bytes calldata _toAddress,                  // the receiver of the destination ETH
           uint256 _amountLD,                          // the amount, in Local Decimals, to be swapped
           uint256 _minAmountLD                        // the minimum amount accepted out on destination
       ) external payable {
           require(msg.value > _amountLD, "Stargate: msg.value must be > _amountLD");
   
           // wrap the ETH into WETH
           IStargateEthVault(stargateEthVault).deposit{value: _amountLD}();
           IStargateEthVault(stargateEthVault).approve(address(stargateRouter), _amountLD);
   
           // messageFee is the remainder of the msg.value after wrap
           uint256 messageFee = msg.value - _amountLD;
   
           // compose a stargate swap() using the WETH that was just wrapped
           stargateRouter.swap{value: messageFee}(
               _dstChainId,                        // destination Stargate chainId
               poolId,                             // WETH Stargate poolId on source
               poolId,                             // WETH Stargate poolId on destination
               _refundAddress,                     // message refund address if overpaid
               _amountLD,                          // the amount in Local Decimals to swap()
               _minAmountLD,                       // the minimum amount swap()er would allow to get out (ie: slippage)
               IStargateRouter.lzTxObj(0, 0, "0x"),
               _toAddress,                         // address on destination to send to
               bytes("")                           // empty payload, since sending to EOA
           );
       }

2. Stargate’s bridge swap() function will be called. The bridge appends some extra data to the call, and forces the message to be of type TYPE_SWAP_REMOTE. Then, LayerZero’s endpoint will be triggered to deliver the message to the destination chain.

3. In the destination chain, Stargate’s bridgelzReceive() function will be triggered. Because the message sent via the swap is of type TYPE_SWAP_REMOTE, the regular Stargate’s router swapRemote() function will be called. Note how a last payload parameter is passed. This is the parameter that was left empty in the origin chain when ETH router called the regular router’s swap():

   // Stargate's Bridge.sol
   
   function lzReceive(
           uint16 _srcChainId,
           bytes memory _srcAddress,
           uint64 _nonce,
           bytes memory _payload
       ) external override {
           ...
           uint8 functionType;
           assembly {
               functionType := mload(add(_payload, 32))
           }
   
           if (functionType == TYPE_SWAP_REMOTE) {
               ...
               router.swapRemote(_srcChainId, _srcAddress, _nonce, srcPoolId, dstPoolId, dstGasForCall, toAddress, s, payload);
   
           ...
       }

4. Finally, the regular router’s swapRemote() is called, which will call the internal _swapRemote() function. Inside this function, two important steps will take place:

   1. Assets received will be transferred to the destination address _to ( in Tapioca’s flow it will be the mtOFT contract).
   2. if _payload.length > 0 , then destination address _to sgReceive() function will be called. This is the step that should be called in order to deposit the assets from the mtOFT to the TOFTVault. However, this step will never be triggered when routing native assets, because as we saw in the first step, Stargate’s ETH router will hardcode the payload to be empty, making the _payload.length be 0, and preventing the mtOFT’s sgReceive() function from being called.

   function _swapRemote(
           uint16 _srcChainId,
           bytes memory _srcAddress,
           uint256 _nonce,
           uint256 _srcPoolId,
           uint256 _dstPoolId,
           uint256 _dstGasForCall,
           address _to,
           Pool.SwapObj memory _s,
           bytes memory _payload
       ) internal {
           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 {
               revertLookup[_srcChainId][_srcAddress][_nonce] = abi.encode(
                   TYPE_SWAP_REMOTE_RETRY,
                   _srcPoolId,
                   _dstPoolId,
                   _dstGasForCall,
                   _to,
                   _s,
                   _payload
               );
               emit Revert(TYPE_SWAP_REMOTE_RETRY, _srcChainId, _srcAddress, _nonce);
           }
       }

Because sgReceive() is never called, bridged nativeassets will remain stuck in the mtOFT contract when received in the destination chain. Then, an attacker can use the wrap() function in order to steal all the idle native assets stuck in the contract.

Because inside the wrap() mtOFT function the msg.value is not checked against the _amount that is actually passed as parameter, an attacker can pass a small msg.value amount enough to cover the wrapping fees but pass an _amount parameter equal to the amount of idle native assets, effectively wrapping and getting minted all the stuck bridged amount of native assets.

// mtOFT.sol

function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        returns (uint256 minted)
    {
        ...

        uint256 feeAmount = _checkAndExtractFees(_amount);
        if (erc20 == address(0)) {
            _wrapNative(_toAddress, _amount, feeAmount);
        ...
    } 

// Balancer.sol

function _wrapNative(address _toAddress, uint256 _amount, uint256 _feeAmount) internal virtual {
        vault.depositNative{value: _amount}();
        _mint(_toAddress, _amount - _feeAmount);
    }

Note: this issue does not happen when ERC20 tokens are rebalanced because the Balancer contract hardcodes a "0x" in the payload field for ERC20s. This makes the payload length be 2 instead of 0 and forces the Stargate router in the destination chain to call sgReceive():

// Balancer.sol

function _routerSwap(
        uint16 _dstChainId,
        uint256 _srcPoolId,
        uint256 _dstPoolId,
        uint256 _amount,
        uint256 _slippage,
        address payable _oft,
        address _erc20
    ) private {
        bytes memory _dst = abi.encodePacked(connectedOFTs[_oft][_dstChainId].dstOft);
        IERC20(_erc20).safeApprove(address(router), _amount);
        router.swap{value: msg.value}(
            _dstChainId, 
            _srcPoolId,
            _dstPoolId, 
            payable(this),
            _amount,
            _computeMinAmount(_amount, _slippage),
            IStargateRouterBase.lzTxObj({dstGasForCall: 0, dstNativeAmount: 0, dstNativeAddr: "0x0"}),
            _dst,
            "0x"  // <----- This makes the `_payload.length > 0` check performed by Stargate's router pass
        );
    }

Impact

High. All native funds bridged via the Balancer contract wan be stolen by an attacker.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/Balancer.sol#L272-L278

Tool used

Manual Review

Recommendation

One possible solution is to use the regular router to bridge native assets, and wrap them to their wrapped version prior to interacting with the router.

On the other hand, there are some chains where Stargate has deployed ETH routers that allow custom payloads These routers incorporate a swapETHAndCall() function that allows to directly interact with the native asset and does not hardcode the payload to be an empty string. Although limited to some specific chains, using these routers could be another solution.

Duplicate of #69

[0xadrii]

Escalate I believe this issue should clearly be a high and not a medium. Both this report as well as issue 69 clearly show how a substantial loss of funds is possible. As per Sherlock's criteria to identify a high issue, issues must be considered of high impact if a "Definite loss of funds without (extensive) limitations of external conditions." takes place. For this specific issue, no external conditions or limitations exist, the issue will always be present in the contract and all the ETH can effectively be stolen when a rebalance is triggered for mtOFTs.

[sherlock-admin2]

Escalate I believe this issue should clearly be a high and not a medium. Both this report as well as issue 69 clearly show how a substantial loss of funds is possible. As per Sherlock's criteria to identify a high issue, issues must be considered of high impact if a "Definite loss of funds without (extensive) limitations of external conditions." takes place. For this specific issue, no external conditions or limitations exist, the issue will always be present in the contract and all the ETH can effectively be stolen when a rebalance is triggered for mtOFTs.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[nevillehuang]

@0xadrii From discussion in #69, for the planned stargateComposer to be used, wouldn't it simply cause an revert?

[0xadrii]

As windhustler mentions in this comment, the StargateComposer contract does not have a swapETH() function (it rather has a swapETHAndCall() function), so the stargate composer contract can't be the one that was expected to be used.

The tests also clearly show that the intention was to use the ETH router (forking with mainnet address 0x150f94b44927f078737562f0fcf3c95c01cc2376) rather than using the Stargate composer, so it is clear that the intention to handle native ETH was to use the ETH router.

Setting the ETH router as the contract, the functions won't revert, and the cross-chain call will actually take place. However, as mentioned in my report and windhustler's, because sgReceive() is never called on the destination chain the transferred ETH will remain stuck in the wrong contract (mtOFT instead of the vault), and users can drain the bridged ETH by leveraging the fact that the wrap() function does not check that the _amount passed as parameter is equal to msg.value, so users can wrap the stuck ETH as if it was theirs.

[nevillehuang]

@0xadrii Fair enough, I think high severity is warranted. Thanks for the detailed escalation

[cvetanovv]

I agree with the escalation. It meets the criteria of High severity.

[cvetanovv]

Planning to accept the escalation and make this issue a valid High.

[Evert0x]

Result: High Duplicate of #69

[sherlock-admin3]

Escalations have been resolved successfully!

Escalation status:

• 0xadrii: accepted