The `Sender` of an outbound cctx originating from the zEVM is potentially set to an incorrect sender address resulting in lost assets during a refund

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/evm_hooks.go#L141 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/evm_hooks.go#L216

Vulnerability details

Impact

If an intermediary contract is used on the zEVM when interacting with the ZetaConnectorZEVM contract or ZRC-20 tokens, potential refunds are sent to the transaction signer instead of the intermediary contract, allowing users to steal assets by crafting a cctx that fails and refunds the assets to them.

Determining the validity of the message call via the ZetaInteractor.isValidMessageCall modifier is unreliable.

Proof of Concept

zEVM transactions are post-processed in the PostTxProcessing function of the x/crosschain module. Specifically, the goal is to parse and process ZetaSent and ZRC-20 Withdrawal events and send them to the corresponding, external receiver chains.

Any emitted ZetaSent events are parsed and processed in the ProcessZetaSentEvent function. Similarly, ZRC-20 Withdrawal events are processed in the ProcessZRC20WithdrawalEvent function.

Both functions create a MsgVoteOnObservedInboundTx message in lines 139-154 and 214-228.

However, the Sender is set to the emittingContract address in line 141 and 216, which is the address of the transaction signer (i.e., sender - tx.origin). In the very special case that the transaction is a contract creation transaction, the To address is set to nil, and thus the Sender is set to nil as well.

In both cases, it is problematic as the cctx's Sender is used in case the outbound cctx is reverted and will be set as the receiver of the refund. As a result, the refund is potentially sent to the wrong recipient.

Moreover, the Sender is provided to the ZetaConnectorEth.onReceive (or non-eth connector) function as the zetaTxSenderAddress address. This sender address can be used to implement access protection on the receiving contract by using the ZetaInteractor.isValidMessageCall modifier. However, as the zetaTxSenderAddress is unreliably set (always to the transaction signer's address), this limits the functionality and prevents the use of an intermediary contract on the zEVM.

Similarly, in the HandleEVMDeposit function, calling the ProcessLogs function to process any logs that are a result of a zEVM deposit, and providing the msg.Receiver (i.e., to) as the emittingContract address, results in similar issues.

Tl;dr

Using an intermediary contract on the zEVM that interacts with the ZetaConnectorZEVM contract causes refund issues and access protection issues on the receiving contract.

Additionally, interacting with the ZetaConnectorZEVM contract on the zEVM, as part of a contract creation transaction, is discouraged and leads to lost assets when attempting to refund a failed cctx. This should be clearly documented to prevent users from losing assets.

Tools Used

Manual review

Recommended mitigation steps

Consider using the zetaTxSenderAddress address of the ZetaSent event as well as the from address of the Withdrawal event as the Sender of the cctx.

Assessed type

Other

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #408

[c4-pre-sort]

DadeKuma marked the issue as sufficient quality report

[c4-judge]

0xean marked the issue as satisfactory

[berndartmueller]

This submission should not be considered a duplicate of #408, as the affected code is different. Here, the root cause lies within the ZEVM, in evm_hooks.go#L141 and evm_hooks.go#L216, where the Sender is incorrectly set.

In contrast, #408 is about the zetaclient, specifically, utils.go#L123, incorrectly setting the Sender of an inbound in the GetInboundVoteMsgForDepositedEvent function.

Additionally, #285 should be duped to #407 instead of #408.

Thanks for having a second look on this!

[0xean]

agreed

[c4-judge]

0xean marked the issue as not a duplicate

[c4-judge]

0xean marked the issue as primary issue

[c4-judge]

0xean marked the issue as selected for report

[c4-sponsor]

lumtis (sponsor) confirmed