Users underpay the fee cost of the transactions in the case of using _hasFallbackToggled that is set to true when using the Branch Bridge Agent and Root Bridge Agent

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L785-L795 https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L938-L948 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L464 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L343 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L281 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L311 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L468 https://github.com/code-423n4/2023-09-maia/blob/main/src/BranchBridgeAgent.sol#L348 https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L182 https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L209 https://github.com/code-423n4/2023-09-maia/blob/main/src/RootBridgeAgent.sol#L238

Vulnerability details

Impact

The fees calculated is underpaid because the fee of the Fallback transaction is not taken into the calculation. Users can estimate the fees using getFeeEstimate.
In case the _hasFallbackToggled is true and the protocol need to send back the fallback message via the LayerZero then the users underpaid the amount of the message fee for the fallback message. Because users 's transactions will pass if the msg.value is enough to cover the message fees for delivering the message on the destination chain. The LayerZero Endpoint message fees is calculated based only on the message that need to be sent via the EndPoint. Any amount (msg.value - message fee) is refunded to the user (the refundee).

Currently the contracts RootBridgeAgent have getFeeEstimate as following:

Title

/// @inheritdoc IBranchBridgeAgent
    function getFeeEstimate(uint256 _gasLimit, uint256 _remoteBranchExecutionGas, bytes calldata _payload)
        external
        view
        returns (uint256 _fee)
    {
        (_fee,) = ILayerZeroEndpoint(lzEndpointAddress).estimateFees(
            rootChainId,
            address(this),
            _payload,
            false,
            abi.encodePacked(uint16(2), _gasLimit, _remoteBranchExecutionGas, rootBridgeAgentAddress)
        );
    }

Title

/// @inheritdoc IRootBridgeAgent
    function getFeeEstimate(
        uint256 _gasLimit,
        uint256 _remoteBranchExecutionGas,
        bytes calldata _payload,
        uint16 _dstChainId
    ) external view returns (uint256 _fee) {
        (_fee,) = ILayerZeroEndpoint(lzEndpointAddress).estimateFees(
            _dstChainId,
            address(this),
            _payload,
            false,
            abi.encodePacked(uint16(2), _gasLimit, _remoteBranchExecutionGas, getBranchBridgeAgent[_dstChainId])
        );
    }

So the Fee calculation is based on the _payload of the transaction sent from the other chains to the root chain and vice versa. But in the case, the _hasFallbackToggled is true, the cost of sending the Fallback transaction is not charged to the user and this will charge from the contract balance.

Note: Currently the Maia protocol supports the Fallback feature that will send the fallback payload. In the Branch Bridge Agent contract, the fallback payload will be sent via the Layerzero to the Root Bridge Agent as seen in the code below.

When the _hasFallbackToggled is true, then in some scenarios, the bridge agent will send the fallback payload to other chains as seen below.

Title

/**
     * @notice Internal function performs the call to Layerzero Endpoint Contract for cross-chain messaging.
     *   @param _refundee address to refund gas to.
     *   @param _settlementNonce root settlement nonce to fallback.
     */
    function _performFallbackCall(address payable _refundee, uint32 _settlementNonce) internal virtual {
        //Sends message to LayerZero messaging layer
        ILayerZeroEndpoint(lzEndpointAddress).send{value: address(this).balance}(
            rootChainId,
            rootBridgeAgentPath,
            abi.encodePacked(bytes1(0x09), _settlementNonce),
            _refundee,
            address(0),
            ""
        );
    }

In the Root Bridge Agent contract, the fallback payload will be sent via the Layerzero to the Branch Bridge Agent as seen in the code below. Title

/**
     * @notice Internal function performs call to Layerzero Enpoint Contract for cross-chain messaging.
     *   @param _depositNonce branch deposit nonce.
     *   @param _dstChainId Chain ID of destination chain.
     */
    function _performFallbackCall(address payable _refundee, uint32 _depositNonce, uint16 _dstChainId) internal {
        //Sends message to LayerZero messaging layer
        ILayerZeroEndpoint(lzEndpointAddress).send{value: address(this).balance}(
            _dstChainId,
            getBranchBridgeAgentPath[_dstChainId],
            abi.encodePacked(bytes1(0x04), _depositNonce),
            payable(_refundee),
            address(0),
            ""
        );
    }

Proof of Concept

Step 1: Users call on the Branch Bridge Agent or the Root Bridge Agent with _hasFallbackToggled is true for example

function callOutSignedAndBridge(
        address payable _refundee,
        bytes calldata _params,
        DepositInput memory _dParams,
        GasParams calldata _gParams,
        bool _hasFallbackToggled
    ) 

_hasFallbackToggled is true

Step 2: In case there is some error in the root chain, the fallback message is sent to the Branch Bridge Agent, then users call the redeemDeposit to take back his money.

The POC on Branch Bridge Agent is similar to test case testFallbackClearDepositRedeemSuccess Title

function testFallbackClearDepositRedeemSuccess() public {
        // Create Test Deposit
        testCallOutSignedAndBridge(address(this), 100 ether);

        vm.deal(localPortAddress, 1 ether);

        // Encode Fallback message
        bytes memory fallbackData = abi.encodePacked(bytes1(0x04), uint32(1));

        // Call 'Fallback' -> This simulates the root Bridge Agent sent the Fallback payload to the Branch Bridge Agent
        vm.prank(lzEndpointAddress);
        bAgent.lzReceive(rootChainId, abi.encodePacked(bAgent, rootBridgeAgentAddress), 1, fallbackData);

        // Call redeemDeposit
        bAgent.redeemDeposit(1);

        // Check deposit state
        require(bAgent.getDepositEntry(1).owner == address(0), "Deposit should be deleted");

        // Check balances
        require(testToken.balanceOf(address(this)) == 0);
        require(underlyingToken.balanceOf(address(this)) == 100 ether);
        require(testToken.balanceOf(localPortAddress) == 0);
        require(underlyingToken.balanceOf(localPortAddress) == 0);
    }

The flow of the message on the root chain: the message is sent from the Layerzero Endpoint to the Root bridge agent Title

else if (_payload[0] & 0x7F == 0x05) {
            // Parse deposit nonce
            nonce = uint32(bytes4(_payload[PARAMS_START_SIGNED:PARAMS_TKN_START_SIGNED]));

            //Check if tx has already been executed
            if (executionState[_srcChainId][nonce] != STATUS_READY) {
                revert AlreadyExecutedTransaction();
            }

            // Get User Virtual Account
            VirtualAccount userAccount = IPort(localPortAddress).fetchVirtualAccount(
                address(uint160(bytes20(_payload[PARAMS_START:PARAMS_START_SIGNED])))
            );

            // Toggle Router Virtual Account use for tx execution
            IPort(localPortAddress).toggleVirtualAccountApproved(userAccount, localRouterAddress);

            // Avoid stack too deep
            uint16 srcChainId = _srcChainId;

            // Try to execute remote request
            // Flag 5 - RootBridgeAgentExecutor(bridgeAgentExecutorAddress).executeSignedWithDeposit(address(userAccount), localRouterAddress, data, _srcChainId)
            _execute(
                _payload[0] == 0x85,
                nonce,
                address(uint160(bytes20(_payload[PARAMS_START:PARAMS_START_SIGNED]))),
                abi.encodeWithSelector(
                    RootBridgeAgentExecutor.executeSignedWithDeposit.selector,
                    address(userAccount),
                    localRouterAddress,
                    _payload,
                    srcChainId
                ),
                srcChainId
            );

            // Toggle Router Virtual Account use for tx execution
            IPort(localPortAddress).toggleVirtualAccountApproved(userAccount, localRouterAddress);

            // DEPOSIT FLAG: 6 (Call with multiple asset Deposit + msg.sender)
        }

Then Title

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

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

        //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();
            }
        }
    }

Then Title

function _performFallbackCall(address payable _refundee, uint32 _depositNonce, uint16 _dstChainId) internal {
        //Sends message to LayerZero messaging layer
        ILayerZeroEndpoint(lzEndpointAddress).send{value: address(this).balance}(
            _dstChainId,
            getBranchBridgeAgentPath[_dstChainId],
            abi.encodePacked(bytes1(0x04), _depositNonce),
            payable(_refundee),
            address(0),
            ""
        );
    }

The cost of the fallback message is charged from the contract balance.

---

Note: The contract balance ETH in Root Bridge Agent and Branch Bridge Agent can come from the Layerzero endpoint when users uses adapter parameters: _gasParams.remoteBranchExecutionGas as airdrop native token via Layerzero as explained in (https://layerzero.gitbook.io/docs/evm-guides/advanced/relayer-adapter-parameters). We can see this is similuted in the test cases of Maia: https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/test/ulysses-omnichain/MulticallRootRouterTest.t.sol#L1384-L1386

But notice that in the lzReceive of Root Bridge Agent and Branch Bridge Agent, it is used excessivelySafeCall to lzReceiveNonBlocking, so if the call to lzReceiveNonBlocking is failed (reverted), the message delivery from the Layerzero endpoint is still succesfull. So for some transactions that the call to lzReceiveNonBlocking is reverted, it does not spend ETH, but the contract still received ETH from Layerzero, so ETH is accumulated in Root Bridge Agent and Branch Bridge Agent. This ETH can come from different users with different transactions.

End of Note.

---

Tools Used

Manual Review

Recommended Mitigation Steps

Should calculate the messageFee and in case the fallback is needed then the message fee should include the fee of sending the fallback. If not using the fallback, should refund the users on the other chain.

Assessed type

ETH-Transfer

[c4-pre-sort]

0xA5DF marked the issue as sufficient quality report

[c4-pre-sort]

0xA5DF marked the issue as primary issue

[0xA5DF]

Severity shouldn't be more than med imo, since it's probably not that common and the core assets are not lost

[c4-sponsor]

0xLightt (sponsor) disputed

[0xLightt]

The behavior described, where a transaction failure results in native tokens being retained in the Bridge Agents and potentially utilized by the next user, is by design. This system reduces complexity in our gas management process. The recommendation to calculate the messageFee in advance and to include potential fallback costs is based on a misunderstanding. Implementing such a mechanism would, in most cases, be misleading. Since this estimate is from the source to the destination and not from the destination to the source as the fallback would be. Refunding users would be dangerous and could brick the BridgeAgent by setting it's layerzero endpoint status to blocking.

[alcueca]

The sponsor acknowledges the issue. Accepting that users will sometimes lose value might be an acceptable trade-off for the sponsor, but that doesn't negate the issue as per the rules of the contest.

Given that the losses are limited to a part of the gas the user accepted to use for the transaction, the severity is Medium.

[c4-judge]

alcueca changed the severity to 2 (Med Risk)

[alcueca]

I believe there might be a number of duplicates to this issue.

[c4-judge]

alcueca marked the issue as satisfactory

[alcueca]

The issue is that getFeeEstimate doesn't include the gas costs of a fallback call, if it were to happen. The only costs to the user are gas costs from a transaction reverting in very rare cases. The agent should not have native tokens, and if they do, it is accepted that anyone can take them. I can't really classify this as more than QA.

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a