Incorrect genesis initialization of pending nonces

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/genesis.go#L63-L64

Vulnerability details

Impact

In case a ZetaChain node has been hard-reset or a hard fork is performed, the x/crosschain module's InitGenesis function will initialize the pending nonces for all external chains to 0, diverging from the real pending, non-zero, nonces. Consequently, assigning the next available transaction nonce to a cctx via the UpdateNonce function will error, preventing the cctx from ever being processed.

Proof of Concept

The InitGenesis function in node/x/crosschain/genesis.go#L12-L73 initializes the chain nonces in lines 39-41 from the exported state. These nonces are possibly non-zero as they represent the next available transaction nonce for the chains.

38: // Set all the chain nonces
39: for _, elem := range genState.ChainNoncesList {
40:     if elem != nil {
41:         k.SetChainNonces(ctx, *elem)
42:     }
43: }

Additionally, the pending nonces are initialized in lines 63-64, by setting the NonceLow and NonceHigh to 0.

59: if genState.Tss != nil {
60:     k.SetTSS(ctx, *genState.Tss)
61:     for _, chain := range common.DefaultChainsList() {
62:         k.SetPendingNonces(ctx, types.PendingNonces{
63: ❌           NonceLow:  0,
64: ❌           NonceHigh: 0,
65:             ChainId:   chain.ChainId,
66:             Tss:       genState.Tss.TssPubkey,
67:         })
68:     }
69:     for _, elem := range genState.TssHistory {
70:         k.SetTSSHistory(ctx, elem)
71:     }
72: }

However, as a chain's transaction nonce is most probably non-zero (if there have been previous transactions), the pending nonces are initialized incorrectly.

As a result, the UpdateNonce function will error when checking if the NonceHigh nonce is equal to the next available Nonce in node/x/crosschain/keeper/cctx_utils.go#L44-L46 due to the p.NonceHigh being initialized to 0.

44: if p.NonceHigh != int64(nonce.Nonce) {
45:     return cosmoserrors.Wrap(types.ErrNonceMismatch, fmt.Sprintf("chain_id %d, high nonce %d, current nonce %d", receiveChainID, p.NonceHigh, nonce.Nonce))
46: }

Consequently, this error has wide-ranging implications as the UpdateNonce function is called in many instances:

1. Processing EVM transactions via the PostTxProcessing hook. Specifically, the UpdateNonce function is called in node/x/crosschain/keeper/evm_hooks.go#L254 and returns an error. Consequently, the EVM transaction is reverted.
2. Attempting to refund a failed inbound cctx, see node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L180. Consequently, the inbound cctx is aborted without refunding.
3. Cross-chain messaging, see node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L228. As a result, the inbound cross-chain cctx is aborted without refunding the sender or notifying the sender (contract) of the abort.
4. Refunding a failed outbound cctx, see node/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go#L194.
5. Migrating funds from TSS addresses, see node/x/crosschain/keeper/msg_migrate_tss_funds.go#L133.
6. Sending a command cctx to whitelist an ERC-20 token on an external chain. See node/x/crosschain/keeper/msg_server_whitelist_erc20.go#L146

In this scenario, there's currently no functionality to recover from this error. The chain's pending nonces can not be adjusted, and the only way to recover from this is by fixing the issue and having the chain's validators update their nodes.

Test Case

The following simple test case demonstrates that the NonceHigh does not reflect the chain's actual nonce:

Test case (click to reveal)

```diff diff --git a/repos/node/x/crosschain/genesis_test.go b/repos/node/x/crosschain/genesis_test.go index 994c83f..c959de1 100644 --- a/repos/node/x/crosschain/genesis_test.go +++ b/repos/node/x/crosschain/genesis_test.go @@ -1,6 +1,7 @@ package crosschain_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -12,6 +13,8 @@ import ( ) func TestGenesis(t *testing.T) { + chainNonce := int64(2) + genesisState := types.GenesisState{ Params: types.DefaultParams(), OutTxTrackerList: []types.OutTxTracker{ @@ -28,7 +31,7 @@ func TestGenesis(t *testing.T) { ChainNoncesList: []*types.ChainNonces{ sample.ChainNonces(t, "0"), sample.ChainNonces(t, "1"), - sample.ChainNonces(t, "2"), + sample.ChainNonces(t, fmt.Sprintf("%d", chainNonce)), }, CrossChainTxs: []*types.CrossChainTx{ sample.CrossChainTx(t, "0"), @@ -51,6 +54,10 @@ func TestGenesis(t *testing.T) { k, ctx, _, _ := keepertest.CrosschainKeeper(t) crosschain.InitGenesis(ctx, *k, genesisState) got := crosschain.ExportGenesis(ctx, *k) + + pendingNonces, _ := k.GetPendingNonces(ctx, genesisState.Tss.TssPubkey, genesisState.ChainNoncesList[2].ChainId) // @audit-info 3rd chain with the nonce set to 2 + require.Equal(t, chainNonce, pendingNonces.NonceHigh) // @audit-info fails, expected 2, got 0 + require.NotNil(t, got) // Compare genesis after init and export ``` ```sh --- FAIL: TestGenesis (0.01s) ./2023-11-zetachain/repos/node/x/crosschain/genesis_test.go:59: Error Trace: ./2023-11-zetachain/repos/node/x/crosschain/genesis_test.go:59 Error: Not equal: expected: 2 actual : 0 Test: TestGenesis ``` **How to run this test case:** Save git diff to a file in the root named `test.patch` and run with ```bash git apply test.patch cd repos/node go test -v ./x/crosschain/genesis_test.go --run TestGenesis ``` **Result:** ```bash === RUN TestGenesis genesis_test.go:59: Error Trace: /Users/bernd/Projects/crypto/audits/c4-2023-11-zetachain/repos/node/x/crosschain/genesis_test.go:59 Error: Not equal: expected: 2 actual : 0 Test: TestGenesis --- FAIL: TestGenesis (0.01s) ```

Tools Used

Manual review

Recommended mitigation steps

Consider exporting the pending nonces in the ExportGenesis function and initialize the pending nonces state from it in the InitGenesis function.

Assessed type

Invalid Validation

[c4-pre-sort]

DadeKuma marked the issue as sufficient quality report

[c4-pre-sort]

DadeKuma marked the issue as primary issue

[c4-sponsor]

lumtis (sponsor) confirmed

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean marked the issue as selected for report