Description

One of the responsibilities of the zeta client is to watch for incoming transactions and handle any ZetaSent events emitted by the connector.

logs, err := ob.Connector.FilterZetaSent(&bind.FilterOpts{
	Start:   uint64(startBlock),
	End:     &tb,
	Context: context.TODO(),
}, []ethcommon.Address{}, []*big.Int{})

if err != nil {
	return err
}
cnt, err := ob.GetPromCounter("rpc_getLogs_count")
if err != nil {
	return err
}
cnt.Inc()

// Pull out arguments from logs
for logs.Next() {
	event := logs.Event
	ob.logger.Info().Msgf("TxBlockNumber %d Transaction Hash: %s Message : %s", event.Raw.BlockNumber, event.Raw.TxHash, event.Message)
	destChain := common.GetChainFromChainID(event.DestinationChainId.Int64())
	destAddr := clienttypes.BytesToEthHex(event.DestinationAddress)

	if strings.EqualFold(destAddr, config.ChainConfigs[destChain.ChainName.String()].ZETATokenContractAddress) {
		ob.logger.Warn().Msgf("potential attack attempt: %s destination address is ZETA token contract address %s", destChain, destAddr)}

When fetching the destination chain, common.GetChainFromChainID(event.DestinationChainId.Int64()) is used, which will return nil if the chain is not found.

func GetChainFromChainID(chainID int64) *Chain {
	chains := DefaultChainsList()
	for _, chain := range chains {
		if chainID == chain.ChainId {
			return chain
		}
	}
	return nil
}

Since a user is able to specify any value for the destination chain, if a nonsupported chain is used, then destChain will be nil and the following destChain.ChainName call will cause the client to crash.

Impact

As all the clients watching the remote chain will see the same events, a malicious user (or a simple mistake entering the chain) will cause all the clients to crash. If the clients automatically restart and try to pick up from the block they were up to (the default), then they will crash again and enter into an endless restart and crash loop. This will prevent any incoming or outgoing transactions on the remote chain from being processed, effectively halting that chain's integration.

Recommendations

There should be an explicit check to ensure that destChain is not nil and to skip the log if it is.

It would also be a good idea to have a recovery mechanism that can handle any blocks that cause the client to crash and skip them. This will help prevent the remote chain from being paused if a similar bug occurs again.

Remediation

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