Closed c4-submissions closed 12 months ago
0xA5DF marked the issue as duplicate of #877
0xA5DF marked the issue as sufficient quality report
alcueca marked the issue as not a duplicate
alcueca marked the issue as duplicate of #679
alcueca marked the issue as satisfactory
alcueca marked the issue as selected for report
From the sponsor:
Contracts should not use Virtual Accounts and deploying a specific router for contract usage is most likely the safest option.
alcueca changed the severity to 2 (Med Risk)
This issue only talks about native tokens, and not deposits.
alcueca marked the issue as not selected for report
alcueca marked issue #872 as primary and marked this issue as a duplicate of 872
Issue #872 highlight
the same address for multisig in different network can belong to different owner
but don't see this report make such point so don't see this is a duplicate of #872, I wonder if this issue can be duplicate with #679 together
alcueca marked the issue as not a duplicate
alcueca marked the issue as duplicate of #679
While not a 100% duplicate of #679, because of pointing at multiple lines with similar errors, the underlying bug is the same, including the same line as #679. The impact described is lower, and I see more appropriate to mark #679 as best.
alcueca changed the severity to 3 (High Risk)
Lines of code
https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L713 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L164 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L188 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L255 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L288 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L344 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L377 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L432 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/MulticallRootRouter.sol#L465
Vulnerability details
The LayerZero has in place a mechanism for refunding extra native tokens provided to their endpoint's
send
method that relies on the caller (bridge agent contracts in this case) providing a refund address.The refundee addresses provided the multipla instances of
send
calls in BranchBridgeAgent and RootBridgeAgent seem sound, except for a few that are problematic.BranchBridgeAgent.retrieveDeposit
use case, both messages (retrieve deposit and fallback) are refunded to theretrieveDeposit
caller (msg.sender
). While this is correct on the branch chain, on the root chain it may not.MulticallRootRouter
'smulticallSingleOutput
andmulticallMultipleOutput
(signed), native tokens are refunded in the root chain to the owner of the virtual account. While this address is valid on the branch chain that originally created the virtual address, on the root chain it may not.MulticallRootRouter
'smulticallSingleOutput
andmulticallMultipleOutput
(unsigned), native tokens are refunded in the root chain to the output recipient. While this address can be assumed to be valid on the branch chain where the output is directed, on the root chain it may not.For example, if the refundee is a contract that is not deployed on the Arbitrum chain, and it will not be i.e. because its creator has already used the nonce that could deploy at its same address, the refund native tokens are lost.
Impact
Users who can't sign transactions from the same address on the branch and root chain may end up overpaying thir calls.
Proof of Concept
The following runnable (foundry) PoC shows how the root chain endpoint sends tokens to an address (
contractUser
) that is guaranteed to be valid only on the branch chain, proving the impact on the first scenario:Tools Used
Code review, Foundry
Recommended Mitigation Steps
Possible options would be:
retrieveDeposit
callor, more accurately:
GasParams
to include the address for native tokens refundees, so the refundee can be different at every hopGasParams
argument to the fallback calls tooAssessed type
Token-Transfer