c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L276 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L577

Vulnerability details

Impact

Smart Contract calling callOutSignedAndBridge via BranchBridgeAgent can cause loss of fun

Proof of Concept

One of the cross-chain request pass is that when user calling callOutSignedAndBridge via BranchBridgeAgent

the payload is created

    //Encode Data for cross-chain call.
    bytes memory payload = abi.encodePacked(
        _hasFallbackToggled ? bytes1(0x85) : bytes1(0x05),
        msg.sender,
        _depositNonce,
        _dParams.hToken,
        _dParams.token,
        _dParams.amount,
        _dParams.deposit,
        _params
    );

this would trigger the code on RootBridgeAgent.sol

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

the msg.sender address in source chain (branch bridge agent chain) will be used to either fetch or create a new virtual wallet

   /// @inheritdoc IRootPort
    function fetchVirtualAccount(address _user) external override returns (VirtualAccount account) {
        account = getUserAccount[_user];
        if (address(account) == address(0)) account = addVirtualAccount(_user);
    }

and this function has no access control, anyone can trigger this function to create a virtual account for specific user address

and the function will not revert, if wallet does not exist, wallet is created for the user.

the code assume the same address in different blockchain belongs to the same owner

this is mostly true for EOA account

but not true for smart contract address (for example, multisig)

the same address for multisig in different network can belong to different owner

for example https://rekt.news/wintermute-rekt/

the false assumption of a mutlisig smart contract address is controlled by same owner in different network has cost 20M OP lost

the multisig address is controlled by wintermute in mainnet

the attacker observe the OP is sent to the same address in OP network

the attacker manage the get the factory nonce of the gensis safe factory and redeploy the same address in OP network to control the OP token

this could happens to the current implementation of maia dao

a user can observer when a multisig address trigger callOutSignedAndBridge and redeploy the same address in different network (root bridge agent) to control the fund

or it is possible a smart contract in blockchain A does not belong to anyone in blockchain B, in that case, the fund is lost

Tools Used

Manual Review

Recommended Mitigation Steps

let user specify the recipient when calling callOutSignedAndBridge and use the recipient address to fetch virtual account

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as duplicate of #877

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as sufficient quality report