if the Virtual Account's owner is a Contract Account (multisig wallet), attackers can gain control of the Virtual Accounts by gaining control of the same owner's address in a different chain

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/main/src/RootPort.sol#L350-L353

Vulnerability details

Impact

• Attackers can gain control of User's Virtual Accounts and steal all the assets these accounts hold in the Root environment

Proof of Concept

• When sending signed messages from a Branch to Root, the RootBridgeAgent contract calls the RootPort::fetchVirtualAccount() to get the virtual account that is assigned in the Root environment to the address who initiated the call in the SrcBranch, and if that address doesn't have assigned a virtual account yet, it proceeds to create one and assign it.
• The problem is that the fetchVirtualAccount() function solely relies on the address of the caller in the SrcBranch, but it doesn't take into account from which Branch the call comes.

BranchBridgeAgent.sol

function callOutSignedAndBridge(
    ...
) external payable override lock {
  ...
  //Encode Data for cross-chain call.
  bytes memory payload = abi.encodePacked(
      _hasFallbackToggled ? bytes1(0x85) : bytes1(0x05),
      //@audit-info => Encodes the address of the caller in the Branch and sends it to the RootBridgeAgent
      //@audit-info => This address will be used to fetch the VirtualAccount assigned to it!
      msg.sender,
      _depositNonce,
      _dParams.hToken,
      _dParams.token,
      _dParams.amount,
      _dParams.deposit,
      _params
  );
}

RootBridgeAgent.sol

function lzReceiveNonBlocking(
  ...

) public override requiresEndpoint(_endpoint, _srcChainId, _srcAddress) {
  ...
  ...
  ...
  else if (_payload[0] == 0x04) {
      // 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();
      }

      //@audit-info => Reads the address of the msg.sender in the BranchBridgeAgent and forwards that address to the RootPort::fetchVirtualAccount()
      // 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);

    ...
    ...
  }
  ...
  ...
}

RootPort.sol

//@audit-info => Receives from the RootBridgeAgent contract the address of the caller in the BranchBridgeAgent contract
//@audit-info => Fetches the VirtualAccount assigned to the _user address regardless from what Branch the call came from
function fetchVirtualAccount(address _user) external override returns (VirtualAccount account) {
    account = getUserAccount[_user];
    if (address(account) == address(0)) account = addVirtualAccount(_user);
}

• Like Example, Let's suppose that a user uses a MultiSigWallet contract to deposit tokens from Avax to Root, in the RootBridgeAgent contract, the address of the MultisigWallet will be used to create a Virtual Account, and all the globalTokens that were bridged will be deposited in this Virtual Account.

  • Now, the problem is that the address of the MultisigWallet, might not be controlled by the same user on a different chain, For example, in Polygon, an attacker could gain control of the address of the same address of the MultisigWallet that was used to deposit tokens from Avax in the Root environment, an attacker can send a signed message from Polygon using the same address of the MultisigWallet that deposited tokens from Avax, to the Root environment, requesting to withdraw the assets that the Virtual Account is holding in the Root environment to the Polygon Branch.
    • When the message is processed by the Root environment, the address that will be used to obtain the Virtual Account will be the address that initiated the call in Polygon, which will be the same address of the user's MultisigWallet contract who deposited the assets from Avax, but the Root environment, when fetching the virtual account, makes no distinctions between the branches, thus, it will give access to the Virtual Account of the attacker's caller address and process the message in the Root environment.
      • As a result, an attacker can gain control of the Virtual Account of an account contract that was used to deposit assets from a chain into Root, by gaining control of the same address of the account contract that deposited the assets in a different chain.

• As explained in detail on this article written by Rekt, it is possible to gain control of the same address for contract accounts in a different chain, especially for those contract accounts that are deployed using the Gnosis Safe contracts:
  

Tools Used

Manual Audit & Article wrote by Rekt

Recommended Mitigation Steps

• The recommendation is to add some logic that validates if the caller address in the BranchBridgeAgent is a contract account or an EOA, and if it's a contract account, send a special flag as part of the crosschain message, so that the RootBridgeAgent contract can know if the caller in the SrcBranch it's a contract or an EOA.
  • If the caller is an EOA, the caller's address can be assigned as the Virtual Account owner on all the chains, for EOAs there are no problems.
  • But, if the caller is a Contract Account, when fetching the virtual account, forward the SrcChain, and if a Virtual Account is created, just authorize the caller address on the SrcBranch as the owner for that Virtual Account, in this way, only the contract account in the SrcBranch can access the Virtual Account in the Root environment.
    • Make sure to use the srcChainId to validate if the caller is an owner of the Virtual Account!

Assessed type

Access Control

[c4-pre-sort]

0xA5DF marked the issue as sufficient quality report

[c4-pre-sort]

0xA5DF marked the issue as primary issue

[c4-sponsor]

0xBugsy (sponsor) disputed

[c4-sponsor]

0xLightt (sponsor) confirmed

[0xLightt]

Contracts should not use Virtual Accounts and deploying a specific router for contract usage is most likely the safest option.

[c4-judge]

alcueca marked the issue as satisfactory

[c4-judge]

alcueca marked the issue as selected for report

[alcueca]

This is related to #351 in the wrong assumption that a given address is controlled by the same individual in all chains. There are different attack vectors and impacts, but fixing that core assumption will solve all of them.

[c4-judge]

alcueca changed the severity to 3 (High Risk)

[c4-judge]

alcueca marked the issue as duplicate of #351

[c4-judge]

alcueca marked the issue as not selected for report

[c4-judge]

alcueca changed the severity to 2 (Med Risk)

[c4-judge]

alcueca marked the issue as selected for report

[alcueca]

Leaving this group as Medium, as this warden estimated it. The underlying issue is assuming that the same addresses are controlled by the same people in different chains, which is not necessarily true. While the sponsor states that contracts should not use virtual accounts, that is not specified in the documentation, and might only have been discovered in a future issue of rekt.

[c4-judge]

alcueca changed the severity to 3 (High Risk)

[khalidfsh]

Greetings all, judge @alcueca

While I appreciate the in-depth analysis presented in the report, there's a fundamental discrepancy when it comes to the exploitability of the vulnerability mentioned.

The report suggests that on Polygon, an attacker could simply gain control of an address identical to the MultiSigWallet from Avax. However, referring to a recent real-world scenario as detailed in the Wintermute incident, we observe that this assumption may be oversimplified.

The Wintermute incident underscores the intricacies and challenges involved in gaining control of a similar address on a different chain:

1. The address would have to be unclaimed or unused on the target chain.
2. Assuming the MultiSigWallet is used across multiple chains, the rightful owners might have already claimed the address.
3. Even if the address is unclaimed, there are intricate steps involving replicating nonce values and other parameters to 'hijack' the address, and this is far from being straightforward.

Given these complexities and the potential for failure, it's crucial to approach the reported vulnerability with a nuanced understanding of its practical exploitability.

Thank you for considering this perspective, and I'd appreciate any further insights on this matter.

[JeffCX]

Agree with the above comments that there maybe some efforts involved to gain control the same address

but the wrong assumption that same address is controlled by same person when using smart contract wallet does cost wintermute lose 20M OP token

so once fund are lost and the lost amount is large

you know in case of wintermute, the multisig wallet is created using the opcode "CREATE" instead of "CREATE2" and the create opcode is not really deprecated, and still be used

cc the original author @stalinMacias

[JeffCX]

more info about the CREATE opcode https://www.evm.codes/#f0?fork=shanghai and deterministic address, https://coinsbench.com/a-comprehensive-guide-to-the-create2-opcode-in-solidity-7c6d40e3f1af

anyway, agree with high severity because the potential lose of fund is large

[alcueca]

And then I upgraded it to High immediately after. I have no idea what happened.

Anyway, there is a significant chance of actual losses because of this vulnerability, that don't need to be enabled by any unlikely prior. The severity is High.

[0xBugsy]

Addressed at Maia-DAO/2023-09-maia-remediations@4df0350