Omniscia Evergon Labs Audit

AccessManagerOmnichainInternal Manual Review Findings

AccessManagerOmnichainInternal Manual Review Findings

AML-01M: Improper Refund Addresses

TypeSeverityLocation
Logical FaultAccessManagerOmnichainInternal.sol:
I-1: L83
I-2: L141
I-3: L162

Description:

The referenced OmnichainProxy function invocations will assume that the refundAddress specified is equivalent across chains or that the caller themselves own the same address on the target chain both of which are incorrect as many L2s employ address masking, especially in the context of smart contract refund recipients (i.e. multi-signature wallets).

Impact:

A cross-chain message's surplus or failure may result in unspent native funds being erroneously sent to an incorrect / inaccessible refund recipient, resulting in minor fund loss.

Example:

contracts/dataIndex/omnichain/AccessManagerOmnichainInternal.sol
62function _approveOmnichainDataManagers(DataPoint dp, OmnichainAddress[] calldata dms, bool approved, address payable refundAddress) internal virtual {
63 (uint32 dpChainId, , ) = DataPoints.decode(dp);
64 ChainidTools.requireCurrentChain(dpChainId);
65 _verifyLocalSenderIsDPAdmin(dp, msg.sender);
66
67 OmnichainProxy proxy = OmnichainSupportStorage.layout().proxy();
68 if (address(proxy) == address(0)) revert ZeroProxyAddress();
69 OmnichainAddress sender = OmnichainAddresses.encode(msg.sender);
70 // Estimate fee
71 (uint256 estimatedTotalFee, uint256[] memory estimatedFees) = _estimateApproveOmnichainDataManagers(proxy, dp, dms, approved, sender);
72
73 // Verify we have enough funds
74 if (msg.value < estimatedTotalFee) {
75 revert NotEnoughFunds(msg.value, estimatedTotalFee);
76 }
77 // Send approvals
78 for (uint256 i; i < dms.length; i++) {
79 OmnichainAddress dm = dms[i];
80 if (estimatedFees[i] == 0) {
81 _approveLocal(dp, dm, approved, sender);
82 } else {
83 proxy.queryApproveDataManager{value: estimatedFees[i]}(dp, dm, approved, sender, REMOTE_APPROVE_DATAMANAGER_GAS_LIMIT, refundAddress);
84 }
85 }
86 // Send refund
87 if (msg.value > estimatedTotalFee) {
88 unchecked {
89 Address.sendValue(refundAddress, msg.value - estimatedTotalFee);
90 }
91 }
92}

Recommendation:

We advise the system to permit a different refund address to be specified per chain and to utilize the caller address as the intended refund recipient for any locally-unspent funds where applicable, ensuring that message refund operations are executed correctly.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The Evergon Labs team evaluated this exhibit and clarified that they do not intend to fix it as they anticipate refunds to solely be processed on the local chain or that the edge cases where the issue would manifest are inconsequential.

After reviewing the relevant GitHub discussions and call breakdowns, we concur with this assessment and thus consider the exhibit safely acknowledged.