Fallback can be DoS by dApps that consumes all gas during multicall

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L778-L793

Vulnerability details

Ulysses allows users to bridge deposit and remotely interact with dApps on the root chain by performing a callOutSignedAndBridge() from branch routers. The user can set _hasFallbackToggled = true, which will perform a fallback call to the source chain when the remote dApps transactions fails (see code below). The fallback call will make the deposit available for redemption on the source chain.

However, it is possible for the dApp remote transactions to consumes all the forwarded gas and revert with an out-of-gas error. When that happens, there will be insufficient gas remaining to trigger the fallback call, causing it to fail as well.

https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L778-L793

    function _execute(
        bool _hasFallbackToggled,
        uint32 _depositNonce,
        address _refundee,
        bytes memory _calldata,
        uint16 _srcChainId
    ) private {
        //Update tx state as executed
        executionState[_srcChainId][_depositNonce] = STATUS_DONE;

        //@audit this is used to call MulticallRootRouter -> VirtualAccount to interact with dApp on root chain
        //Try to execute the remote request
        (bool success,) = bridgeAgentExecutorAddress.call{value: address(this).balance}(_calldata);

        //@audit if the dApp consumes all forwarded gas in the above call(), it will cause the following fallback to fail due to OOG.
        //Update tx state if execution failed
        if (!success) {
            //Read the fallback flag.
            if (_hasFallbackToggled) {
                // Update tx state as retrieve only
                executionState[_srcChainId][_depositNonce] = STATUS_RETRIEVE;
                // Perform the fallback call
                _performFallbackCall(payable(_refundee), _depositNonce, _srcChainId);
            } else {
                // No fallback is requested revert allowing for retry.
                revert ExecutionFailure();
            }
        }
    }

Detailed Explanation

First, lets calculate the remaining gas when the issue occurs:

• if we look at the execution path for branchBridgeAgent.callOutSignedAndBridge(), it will be as follows: Endpoint --> RootBridgeAgent.lzReceive() --> RootBridgeAgent.lzReceiveNonBlocking() --> bridgeAgentExecutorAddress.call() --> RootBridgeAgentExecutor.executeSignedWithDeposit() -->MulticallRootRouter.executeSignedDepositSingle() --> VirtualAccount.call()

• When receiving a cross chain message, LayerZero Endpoint will trigger the 1st external call RootBridgeAgent.lzReceive() with a _gasLimit as shown in Endpoint.sol#L118.

• So at the 1st external call RootBridgeAgent.lzReceive() in RootBridgeAgent.sol#L423, it will be forwarded with a gas value of 63/64 * _gasLimit.

• Following that, the 2nd external call lzReceiveNonBlocking() in RootBridgeAgent.sol#L434, will be forwarded 63/64 63/64 _gasLimit.

• At the 3rd external call bridgeAgentExecutorAddress.call() --> RootBridgeAgentExecutor.executeSignedWithDeposit() in RootBridgeAgent.sol#L779, it will be forwarded a gas of 63/64 63/64 63/64 *_gasLimit.

• Now if dApp interaction at the subsequent call VirtualAccount.call() uses up all the gas, it will revert at the 3rd external call, triggering the fallback. When that happens, it will have 1/64 63/64 63/64 *_gasLimit for _performFallbackCall().

• Assuming a generous _gasLimit of 2M (a uniswapV3 swap is ~200k), the remaining gas for _performFallbackCall() = 1/64 63/64 63/64 * 2M = 30,281.

Now, I have measured the gas required for _performFallbackCall() by adding two lines (see below) and running testRetrySettlementTriggerFallback() in RootForkTest.t.sol. It is estimated to require around ~124,036 gas, which is much higher than what is left when the issue occurs.

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L730-L753

    function _execute(bool _hasFallbackToggled, uint32 _settlementNonce, address _refundee, bytes memory _calldata)
        private
    {
        //Update tx state as executed
        executionState[_settlementNonce] = STATUS_DONE;

        //Try to execute the remote request
        (bool success,) = bridgeAgentExecutorAddress.call{value: address(this).balance}(_calldata);

        //@audit add this 1st line to compute gas required for _performFallbackCall()
        uint256 gasRemaining = gasleft();

        //Update tx state if execution failed
        if (!success) {
            //Read the fallback flag and perform the fallback call if necessary. If not, allow for retrying deposit.
            if (_hasFallbackToggled) {
                // Update tx state as retrieve only
                executionState[_settlementNonce] = STATUS_RETRIEVE;

                // Perform fallback call
                _performFallbackCall(payable(_refundee), _settlementNonce);
            } else {
                // If no fallback is requested revert allowing for settlement retry.
                revert ExecutionFailure();
            }
        }

        //@audit add this 2nd line to compute gas required for _performFallbackCall()
        console.log("gas required for fallback = %d", gasRemaining - gasleft());

Impact

The issue will allow the interacted dApp to consume all gas and cause the fallback to fail. This leads to loss of gas/native tokens for the users, as the user need to perform a retrieve call to redeem the deposit.

Proof of Concept

First add the following for loop to simulate a dApp using up all the forwarded gas and cause the bridgeAgentExecutorAddress.call() to fail and trigger the fallback. Then run testRetrySettlementTriggerFallback() in RootForkTest.t.sol, which will show that the fallback will also fail due to out-of-gas error.

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgentExecutor.sol#L71

    function executeWithSettlement(address _recipient, address _router, bytes calldata _payload)
        external
        payable
        onlyOwner
    {

        //@audit insert the following lines to consume all forwarded gas
        for(uint256 i=0; i<10000; i++) {
            assembly {
                let temp := mload(9999999999999999)
            }            
        }

Note, even though we use BranchBridgeAgentExecutor and BranchBridgeAgent due to how the current test case is constructed, the same issue applies to RootBridgeAgent and RootBridgeAgentExecutor as they are similiar in code.

Recommended Mitigation Steps

Set a gas limit for the bridgeAgentExecutorAddress.call() at RootBridgeAgent.sol#L779 to save some gas for fallback call as shown in the example below.

        //@audit set a gas limit for the forwarded gas, to save some for fallback call 
        (bool success,) = bridgeAgentExecutorAddress.call{gas: gasleft()-200000, value: address(this).balance}(_calldata);

Assessed type

DoS

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #430

[c4-pre-sort]

0xA5DF marked the issue as low quality report

[0xA5DF]

Didn't fully identify the impact as the primary issue

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a

[peakbolt]

I would like to explain that this issue is valid and should not be marked a duplicate of #430 as it describes a different attack vector and impact.

• The attack vector described in 422 is the depletion of forwarded gas by the interacted external dApp, which is different from the return bomb by the trusted contract described in 430. As the interacted dApp is external and not part of Maia Ulysses, it could potentially use up all the forwarded gas, whether intentionally or unintentionally.

• The impact in 422 is the DoS of the fallback feature, which is not mentioned in 430. The purpose of the fallback (when enabled by user), is to allow redemption at the source chain when the cross chain multicall operation fails due to errors/reverts. And this issue shows that the fallback feature will not work as expected when the dApp consumed all forwarded gas causing an revert with out-of-gas error. That will then require users to waste gas token to send another callout to retrieve the deposit.

• This issue will occur even when it is not an user error. One may argue that the users would have tried the transactions in a fork first to avoid this issue, but that would not provide 100% guarantee as there could be unexpected errors (e.g. errors due to re-ordering of transactions, due to MEV opportunities). And handling unexpected errors is precisely what the fallback feature is built for.

• And to add, it is also not a duplicate of 785 and 399. That is because the root cause, impact and attack vector are completely different. To put it simply, even after applying the fix in 785/399, this issue will still occur and the fix is totally different (which is to set a gas limit and reserve sufficient gas for the fallback).

[alcueca]

After discussion with the sponsor, I agree that this is not a duplicate of #430, but given the limited impact (DoS of a single feature, which has an easy alternative for users), the severity remains QA.