Description

The ProcessZetaSentEvent() and ProcessZRC20WithdrawalEvent() functions are used to process ZetaSent and Withdrawal events respectively. These events are emitted by the ZetaConnectorZEVM contract.

These functions first use the parameters of the emitted event to create a new MsgSendVoter message. It then hashes this message and uses the hash as an index to create a new CCTX.

The relevant code in ProcessZetaSentEvent() is shown below:

func (k Keeper) PostTxProcessing(/* ... */) error {
	// [ ... ]

	for _, log := range receipt.Logs {
		// [ ... ]

		eZeta, err := ParseZetaSentEvent(*log)
		if err == nil {
			if err := k.ProcessZetaSentEvent(ctx, eZeta, target, ""); err != nil {
				return err
			}
		}
	}
	return nil
}

func (k Keeper) ProcessZetaSentEvent(ctx sdk.Context, event *contracts.ZetaConnectorZEVMZetaSent, contract ethcommon.Address, txOrigin string) error {
	// [ ... ]

	msg := zetacoretypes.NewMsgSendVoter("", contract.Hex(), senderChain.ChainId, txOrigin, toAddr, receiverChain.ChainId, amount, "", event.Raw.TxHash.String(), event.Raw.BlockNumber, 90000, common.CoinType_Zeta, "")
	sendHash := msg.Digest()
	cctx := k.CreateNewCCTX(ctx, msg, sendHash, zetacoretypes.CctxStatus_PendingOutbound, &senderChain, receiverChain)
	EmitZetaWithdrawCreated(ctx, cctx)
	return k.ProcessCCTX(ctx, cctx, receiverChain)
}

Impact

An issue arises if two or more events are emitted in the same transaction with the same parameters. To demonstrate this, let us assume that two identical ZetaSent events are emitted in the same transaction. If the parameters are the same, then the sendHash that is generated from hashing the MsgSendVoter message will be identical for both the events. When this happens, the CCTX that is created will be the same for both events, and thus the CCTX created for the second ZetaSent event will overwrite the CCTX created for the first ZetaSent event.

An example of a scenario in which this might occur is when a user wants to send 10,000 ZETA tokens to their own address on a different chain. One way they might do this is by opting to send 5,000 ZETA in two ZetaSent events. Since all other parameters would be the same, only the second ZetaSent event gets processed (the CCTX overwrites the first one). This causes the user to only receive 5,000 ZETA on the receiving chain, even though they originally sent 10,000 ZETA.

Additionally, the ProcessCCTX() function will increment the nonce twice in the above scenario. Ethereum enforces that nonces have to always increase by one after each transaction, so in the event that this issue occurs, all outgoing transactions to the receiving chain will begin to fail, halting the bridge in the process.

func (k Keeper) ProcessCCTX(ctx sdk.Context, cctx zetacoretypes.CrossChainTx, receiverChain *common.Chain) error {
	// [ ... ]

	err := k.UpdateNonce(ctx, receiverChain.ChainId, &cctx)
	if err != nil {
		return fmt.Errorf("ProcessWithdrawalEvent: update nonce failed: %s", err.Error())
	}

	// [ ... ]
}

func (k Keeper) UpdateNonce(ctx sdk.Context, receiveChainID int64, cctx *types.CrossChainTx) error {
	chain := k.zetaObserverKeeper.GetParams(ctx).GetChainFromChainID(receiveChainID)

	nonce, found := k.GetChainNonces(ctx, chain.ChainName.String())
	if !found {
		return sdkerrors.Wrap(types.ErrCannotFindReceiverNonce, fmt.Sprintf("Chain(%s) | Identifiers : %s ", chain.ChainName.String(), cctx.LogIdentifierForCCTX()))
	}

	// SET nonce
	cctx.GetCurrentOutTxParam().OutboundTxTssNonce = nonce.Nonce
	nonce.Nonce++
	k.SetChainNonces(ctx, nonce)
	return nil
}

Recommendations

We recommend introducing an ever-increasing nonce within the ZetaConnectorZEVM smart contract. Whenever a new event is emitted by the smart contract, this nonce should be incremented. This means that every emitted event is distinct from all other emitted events, and thus each emitted event will cause the creation of a new CCTX, preventing this issue from occurring.

Remediation

This issue has been acknowledged by ZetaChain, and a fix was implemented in commit 2fdec9ef↗.