Funds from reverted transaction may be lost/locked

mediumCode4rena

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/ZetaConnector.non-eth.sol#L87-L122 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go#L61-L226 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/Zeta.non-eth.sol#L62-L71

Vulnerability details

When the cross chain transaction is reverted on destination chain, the revert transaction is processed on source chain, and the remaining amount of Zeta tokens, excluding fees is returned to the CCTX sender. According to the code comment in /x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go:

If the previous status was PendingOutbound, a new revert transaction is created. To cover the revert transaction fee, the required amount of tokens submitted with the CCTX are swapped using a Uniswap V2 contract instance on ZetaChain for the ZRC20 of the gas token of the receiver chain. The ZRC20 tokens are then burned. The nonce is updated. If everything is successful, the CCTX status is changed to PendingRevert.

If the previous status was PendingRevert, the CCTX is aborted.

The flow is:

1. A cross-chain transaction is generated on the source chain. It's verified and its status is changed to PendingOutbound.
2. The transaction execution is attempted on the destination chain and fails. It is observed by zetaclient/evm_signer/TryProcessOutTx, and broadcased:

go📋
  // [...]
	} else if send.CctxStatus.Status == types.CctxStatus_PendingRevert {
		logger.Info().Msgf("SignRevertTx: %d => %s, nonce %d, gasprice %d", send.InboundTxParams.SenderChainId, toChain, send.GetCurrentOutTxParam().OutboundTxTssNonce, gasprice)
		tx, err = signer.SignRevertTx( // @audit executes ZetaConnector.onRevert()
			ethcommon.HexToAddress(send.InboundTxParams.Sender),
			big.NewInt(send.OutboundTxParams[0].ReceiverChainId),
			to.Bytes(),
			big.NewInt(send.GetCurrentOutTxParam().ReceiverChainId),
			send.GetCurrentOutTxParam().Amount.BigInt(),
			gasLimit,
			message,
			sendhash,
			send.GetCurrentOutTxParam().OutboundTxTssNonce,
			gasprice,
			height,
		)
  // [...]
	if tx != nil {
			// [...]
			err := signer.Broadcast(tx)

3. In case that this is non-eth revert, ZetaConnector.non-eth is called:

    function onRevert(
        address zetaTxSenderAddress,
        uint256 sourceChainId,
        bytes calldata destinationAddress,
        uint256 destinationChainId,
        uint256 remainingZetaValue,
        bytes calldata message,
        bytes32 internalSendHash
    ) external override whenNotPaused onlyTssAddress {
        if (remainingZetaValue + ZetaNonEthInterface(zetaToken).totalSupply() > maxSupply)
            revert ExceedsMaxSupply(maxSupply);
        ZetaNonEthInterface(zetaToken).mint(zetaTxSenderAddress, remainingZetaValue, internalSendHash);

        if (message.length > 0) {
            ZetaReceiver(zetaTxSenderAddress).onZetaRevert(
                ZetaInterfaces.ZetaRevert(
                    zetaTxSenderAddress,
                    sourceChainId,
                    destinationAddress,
                    destinationChainId,
                    remainingZetaValue,
                    message
                )
            );
        }
		// [...]

There are 3 possibilities when it may fail: a) amount to be minted together with totalSupply() is bigger than maxSupply b) ZetaReceiver(zetaTxSenderAddress).onZetaRevert() call reverts c) TSS provides too little gas on purpose, but we assume that it's trused, so it's not really an issue.

If if fails, the transaction state is set to Aborted in x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go, and it won't be processed:

				case types.CctxStatus_PendingRevert:
					cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "Outbound failed: revert failed; abort TX")
				}

In won't be processed again.

And because only Connector can mint the tokens, the funds are lost without anyone able to mint them for the user.

Concerning maxSupply, it's set to uint256.max during contract creation, however it's adjustable by TSS to any value:

javascript📋
    function setMaxSupply(uint256 maxSupply_) external onlyTssAddress {
        maxSupply = maxSupply_;
        emit MaxSupplyUpdated(msg.sender, maxSupply_);
    }

Impact

User funds lost in case that revert transaction reverts on source chain.

Proof of Concept

Let's take following example:

1. User creates a cross chain transaction, to sends 10_000$ worth of ZRC20 cross chain using ZetaChain. He posts a transaction to ZetaConnectorNonEth.send(), which creates cross chain transaction.
2. The transaction is reverted on destination chain. Hence new state PendingRevert is added to the transaction and awaits to be processed on the source chain.
3. ZetaConnectorNonEth.onRevert() is called on source chain. It reverts and cross chain transaction state is set to Aborted.
4. The funds are burned on source chain, and not minted back, meaning that the funds are lost.

Tools Used

Manual analysis

Recommended Mitigation Steps

1. Make sure that TSS cannot decrease maxSupply over constance MIN_SUPPLY_CAP or current totalSupply()

diff📋
    function setMaxSupply(uint256 maxSupply_) external onlyTssAddress {
+		if(maxSupply < MIN_SUPPLY_CAP) {revert MaxSupplyTooSmall()};
        maxSupply = maxSupply_;
        emit MaxSupplyUpdated(msg.sender, maxSupply_);
    }

2. Handle this case gracefully - consider not aborting the transaction in case of revert transaction failure, or add some kind of "voucher" that the user can redeem manually.

Assessed type

Other