Assessment reports>ZetaChain>Critical findings>Bonded validators can trigger reverts for successful transactions
Category: Coding Mistakes

Bonded validators can trigger reverts for successful transactions

Critical Severity
Critical Impact
High Likelihood

Description

A single bonded validator has the ability to add or remove transactions from the out tracker, as the only check is that they are bonded.

func (k msgServer) AddToOutTxTracker(goCtx context.Context, msg *types.MsgAddToOutTxTracker) (*types.MsgAddToOutTxTrackerResponse, error) {
	ctx := sdk.UnwrapSDKContext(goCtx)

	// Zellic: this is the only relevant check
	validators := k.StakingKeeper.GetAllValidators(ctx)
	if !IsBondedValidator(msg.Creator, validators) && msg.Creator != types.AdminKey {
		return nil, sdkerrors.Wrap(sdkerrors.ErrorInvalidSigner, fmt.Sprintf("signer %s is not a bonded validator", msg.Creator))
	}

	// [ ... ]
}

func (k msgServer) RemoveFromOutTxTracker(goCtx context.Context, msg *types.MsgRemoveFromOutTxTracker) (*types.MsgRemoveFromOutTxTrackerResponse, error) {
	ctx := sdk.UnwrapSDKContext(goCtx)

	validators := k.StakingKeeper.GetAllValidators(ctx)
	if !IsBondedValidator(msg.Creator, validators) && msg.Creator != types.AdminKey {
		return nil, sdkerrors.Wrap(sdkerrors.ErrorInvalidSigner, fmt.Sprintf("signer %s is not a bonded validator", msg.Creator))
	}

	k.RemoveOutTxTracker(ctx, msg.ChainId, msg.Nonce)
	return &types.MsgRemoveFromOutTxTrackerResponse{}, nil
}

Impact

This allows a malicious validator to remove an entry from the out transaction tracker and replace it with another one. One way to exploit this would be to

  1. Initiate a Goerli->Goerli message sending some ZETA by calling ZetaConnectorEth.send on the Goerli chain.

  2. After processing the incoming events, a new transaction will be signed, sending the ZETA back to the Goerli chain in signer.TryProcessOutTx and then adding to the outgoing transaction tracker.

  3. The malicious validator can then remove that transaction using tx crosschain remove-from-out-tx-tracker 1337 nonce and add a different transaction that has previously failed (any failed hash will do) using the original nonce.

  4. Then, observeOutTx will pick up this fake transaction from the tracker and add it to ob.outTXConfirmedReceipts and ob.outTXConfirmedTransaction.

  5. Next, IsSendOutTxProcessed is run using this fake receipt and PostReceiveConfirmation is called, marking that status as ReceiveStatus_Failed.

  6. The flow then continues on to revert the cross-chain transactions (CCTXs) and return the ZETA even though the original transaction went through, causing more ZETA to be transferred than was originally sent.

Here is what the attacker's ZETA balance would look like when performing the above attack:

900000000000000000000 // initial balance
890000000000000000000 // balance after triggering ZetaConnectorEth.send
897999999999799398194 // balance after receiving funds from the deleted out tracker tx
903999999999398194581 // balance after receiving the revert funds

Recommendations

Consider whether a single validator should be able to remove transactions from the out tracker or whether it could be done via a vote. If it is unnecessary, then the feature should be removed.

The observeOutTx method could be hardened to ensure that the sender of the transaction is the correct threshold signature scheme (TSS) address and that the nonce of the transaction matches the expected value. This does not prevent a malicious validator from removing legitimate transactions from the tracker and locking up funds.

Remediation

This issue has been acknowledged by ZetaChain, and a fix was implemented in commit's 24d4f9eb and 8222734c.

Zellic © 2024Back to top ↑