User not refunded for failed Zeta gas payment in cross chain transaction

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L169-L185
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L191-L201
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L215-L239

Vulnerability details

When observed CCTX (cross chain transaction) gathers enough votes in VoteOnObservedInboundTx, it is being processed immediately. In case that the transaction is reverted for whatever reason, it returns an error and changes CCTX status to CctxStatus_Aborted - such a transaction won't be processed:

			if err != nil {
				// do not commit anything here as the CCTX should be aborted

				// gas payment for erc20 type might fail because no liquidity pool is defined to swap the zrc20 token into the gas token
				// in this gas we should refund the sender on ZetaChain
				if cctx.InboundTxParams.CoinType == common.CoinType_ERC20 {

					if err := k.RefundAmountOnZetaChain(ctx, cctx, cctx.InboundTxParams.Amount); err != nil {
						// log the error
						k.Logger(ctx).Error("failed to refund amount of aborted cctx on ZetaChain",
							"error", err,
							"sender", cctx.InboundTxParams.Sender,
							"amount", cctx.InboundTxParams.Amount.String(),
						)
					}
				}

				cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error())
				return &types.MsgVoteOnObservedInboundTxResponse{}, nil

There is also no mechanism for retrieving the funds back. TSS could probably initialize such a refund, however it required quorum of nodes to do so, and there is no automatic process for this in code in scope. Possible issues for transaction errors are numerous, I'll list just some of them that immediately come to my head:

• too little amount to cover gas fees. Please note that UniswapV2 is used here, and as described in another issue, it doesn't have proper price manipulation protection, meaning that one of the results may be swapping in highly manipulated pool for overly small fee token amount, making the CCTX error out and abort
• no valid pool for UniswapV2 swap
• too little gas paid to process the transaction
• stale gas prices recorded by keepers
• any issues with gas coin setting in the zetacored

For cross chain swaps it's even worse, because there is no notion of a refund at all, meaning that the funds are frozen:

	// [...]
} else { // Cross Chain SWAP
		tmpCtx, commit := ctx.CacheContext()
		err = func() error {
			err := k.PayGasAndUpdateCctx(
				tmpCtx,
				receiverChain.ChainId,
				&cctx,
				cctx.InboundTxParams.Amount,
				false,
			)
			if err != nil {
				return err
			}
			// [...]
		}()
		if err != nil {
			// do not commit anything here as the CCTX should be aborted
			cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error()) // @audit status changed to "Aborted" without refunding user the money. This refund is partially handled in case of transaction to Zeta
			return &types.MsgVoteOnObservedInboundTxResponse{}, nil
		}

Impact

Temporary or permament freezing of user funds, depending if TSS holders technically are able and willing to refund the funds manually.

Proof of Concept

1. User creates cross chain transaction in ZetaChain
2. The tranasction is reverted, due to Uniswap pool imbalance. Because EVM returns error, the transaction status is changed to Aborted.
3. The user funds are locked.

Tools Used

Manual analysis

Recommended Mitigation Steps

Assessed type

Error

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #245

[c4-pre-sort]

DadeKuma marked the issue as sufficient quality report

[c4-judge]

0xean marked the issue as selected for report

[c4-judge]

0xean marked the issue as satisfactory

[0xean]

sponsor already reviewed in #245

[c4-sponsor]

lumtis (sponsor) confirmed