code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

The attacker can steal all the native coin (ETH) balance of the Root Bridge Agent and the Branch Bridge contracts using the Fallback mechanism #95

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

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

Vulnerability details

Impact

The attacker can steal all the ETH balance of the Root Bridge Agent and the Branch Bridge contracts using the Fallback mechanism.

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.

_performFallbackCall

/**
     * @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. _performFallbackCall

/**
     * @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),
            ""
        );
    }

So in order to support this feature, it is expected that the Root Bridge Agent and the Branch Bridge Agent should have enough ETH to cover the fees to pay the Layerzero Endpoint for the transactions. Otherwise, the transaction will fail. See Message Failed


Note: The 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.


But in this code, the contract send all the ETH balance to the Layerzero Endpoint without calculating the message fee. If the msg.value that is the balance of the contract is bigger than the message fee, then ETH is returned to _refundee and The _refundee is the account that sent the message.

So by using this feature, the attacker can steal all the ETH balance of the Branch Bridge Agent and Root Bridge Agent contracts.

Proof of Concept

Step 1: Attacker call on the Branch Bridge Agent

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

_hasFallbackToggled is true

the _params that have

Step 2: Attacker call the redeemDeposit to take back his money

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

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 _execute

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

The call to bridgeAgentExecutorAddress will return false means failed so the _performFallbackCall is triggered. _performFallbackCall

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),
            ""
        );
    }

So all the ETH balance of the Root Bridge Agent is sent to LayerZero EndPoint, and the refund value is sent to attacker address on the root chain.

Similar attack scenarios can be performed to steal all the ETH of the Branch Bridge Agent

Tools Used

Manual Review

Recommended Mitigation Steps

In the function to performFallback, should calculate the messageFee using the function estimateFees() (https://layerzero.gitbook.io/docs/evm-guides/code-examples/estimating-message-fees) and send only the fee needed.

_performFallbackCall

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),
            ""
        );
    }

https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/src/BranchBridgeAgent.sol#L780-L795

/**
     * @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),
            ""
        );
    }

Assessed type

ETH-Transfer

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #839

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid