Whenever `ArbitrumBranchBridgeAgent` calls `RootBridgeAgent#anyExecute` directly, and deposit fails, `anyFallback` is not called on `ArbitrumBranchBridgeAgent` so as to clear deposits, and allow users to claim their tokens.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L981 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L1005 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L1073 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L1108 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L1134

Vulnerability details

Impact

This causes loss of users funds because when they deposit on ArbitrumBranchBridgeAgent but the deposit was unsuccessful on RootBridgeAgent, the call will not revert(because of try-catch), but rether, returns a "false" success value, and since multicall is not the one that called anyExecute, returning a "false" success value will not trigger anyFallback(which clears users' deposit for redemption) on ArbitrumBranchBridgeAgent.

Proof of Concept

The execution flow for making a deposit from a BranchBridgeAgent on a chain apart from Arbitrum, to RootBridgeAgent goes as follows:

• User deposits by calling one of the available functions e.g. callOutAndBridge
• User's tokens are collected on BranchBridgeAgent
• Multichain's anyCall is used to initiate the cross chain request
• Multichain calls anyExecute on the destination contract
• If anyExecute reverts or returns a "false" success value, anyFallback would be called on source contract, which would allow user to claim their deposits
• Instead of reverting which may expose some attack vectors, protocol wraps the major deposit interaction in a try-catch block, and return false when an error is caught. The assumption is that multichain would call anyFallback in BranchBridgeAgent when it sees that success value is false.

To the best of my understanding, this assumption is correct if BranchBridgeAgent is not ArbitrumBranchBridgeAgent. But for ArbitrumBranchBridgeAgent, RootBridgeAgent#anyExecute is called directly by ArbitrumBranchBridgeAgent, and does not use Multichain's anyCall because both of the contracts are on the same chain. Therefore, returning a "false" success value when ArbitrumBranchBridgeAgent calls RootBridgeAgent will not trigger anyFallback, and users' deposit will not be cleared, but locked forever.

Tools Used

Manual Review

Recommended Mitigation Steps

RootBridgeAgent should call ArbitrumBranchBridgeAgent#anyFallback whenever a deposit from Arbitrum branch was unsuccessful.

Assessed type

Other

[c4-judge]

trust1995 marked the issue as unsatisfactory: Insufficient proof

[Emedudu]

If a deposit from a branch chain does not get recorded on root chain, Multichain calls anyFallback on the branch chain to allow users to claim their funds ArbitrumBranchBridgeAgent also has an anyFallback function which was inherited from BranchBridgeAgent, but this function will not be called when deposits fail in RootBranchBridgeAgent. The reason for this is that the call does not use Multichain because both ArbitrumBranchBridgeAgent and RootBrigdeAgent are on the same chain. So users that make a deposit through ArbitrumBranchBridgeAgent will not be able to claim their deposits if it fails on RootBridgeAgent. The major operations in RootBridgeAgent#anyExecute is wrapped in a try-catch block, and if there is an error(i.e. a failed deposit), a success value of "false" is returned(instead of reverting). When Multichain sees the "false" return value, it calls anyFallback on the source branch chain to clear the user's deposit. But for ArbitrumBranchBridgeAgent, the "false" return value has no effect because it does not trigger anyFallback and does not revert. Therefore user's deposits from ArbitrumBranchBridgeAgent, which fails on RootBridgeAgent, are lost.

[trust1995]

@0xBugsy

[0xBugsy]

If a deposit from a branch chain does not get recorded on root chain, Multichain calls anyFallback on the branch chain to allow users to claim their funds ArbitrumBranchBridgeAgent also has an anyFallback function which was inherited from BranchBridgeAgent, but this function will not be called when deposits fail in RootBranchBridgeAgent. The reason for this is that the call does not use Multichain because both ArbitrumBranchBridgeAgent and RootBrigdeAgent are on the same chain. So users that make a deposit through ArbitrumBranchBridgeAgent will not be able to claim their deposits if it fails on RootBridgeAgent. The major operations in RootBridgeAgent#anyExecute is wrapped in a try-catch block, and if there is an error(i.e. a failed deposit), a success value of "false" is returned(instead of reverting). When Multichain sees the "false" return value, it calls anyFallback on the source branch chain to clear the user's deposit. But for ArbitrumBranchBridgeAgent, the "false" return value has no effect because it does not trigger anyFallback and does not revert. Therefore user's deposits from ArbitrumBranchBridgeAgent, which fails on RootBridgeAgent, are lost.

@trust1995 I believe this is the same as #266, ArbitrumBranchBridgeAgent would have it's anyFallback function called in reaction to a _performCall that failed, due to this we only need to revert execution in such cases since ArbBranch -> Root execution is either started from an EOA or already encapsulated in a try-catch in the Root so reversal is caught and gas properly managed

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as duplicate of #266