An already executed InTxTracker can still be added

mediumCode4rena

Lines of code

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

Vulnerability details

Impact

Anyone can add InTxTracker has been carried out, the node/observer will still deal with executive InTxTracker, Tx will not be repeated(The status of the vote has been changed and the vote cannot pass) ,but an attacker can cause network congestion.

Proof of Concept

A new architecture use Permissionless Tx Validation Model, Anyone can add an InTxTracker, but the AddToInTxTracker function does not verify that an InTxTracker has already been executed, so a Tx that has already been executed can be added again. Since the executed Txs are normal Tx, they can be verified by proof and can be added by anyone.

The observer through GetInboundTrackersForChain access to all the InTxTracker, then parse the Tx content, then MsgVoteOnObservedInboundTx messages sent to a vote, after voting by creates CCTX, Finally execute OutBoundTx.

After the CCTX is created, the InTxTracker will be removed, if added again at this time, the observer still retrieves the Tx and creates a vote, since VoteOnObservedInboundTx uses msg.Digest as the index for voting, so duplicate messages cannot be voted, and eventually CCTX will not be created and will not be executed.

go📋
func (k msgServer) VoteOnObservedInboundTx(....) {
    ....
	cctx := k.CreateNewCCTX(ctx, msg, index, tssPub, types.CctxStatus_PendingInbound, observationChain, receiverChain)
	defer func() {
		EmitEventInboundFinalized(ctx, &cctx)
		// #nosec G701 always positive
		cctx.InboundTxParams.InboundTxFinalizedZetaHeight = uint64(ctx.BlockHeight())
@>		k.RemoveInTxTrackerIfExists(ctx, cctx.InboundTxParams.SenderChainId, cctx.InboundTxParams.InboundTxObservedHash)
		k.SetCctxAndNonceToCctxAndInTxHashToCctx(ctx, cctx)
	}()
}

But here's the problem: failing InTxTracker are not deleted, InTxTracker is deleted after a VoteOnObservedInboundTx vote successfully creates a CCTX, but will not be deleted if a cctx is not created ,they are always available to observers, and they keep on executing and failing, causing congestion when there are a large number of these invalid InTxTracker on the network.

An attacker can add all the executed InTxTracker to the network and attack the network, which can make the network not work.

go📋
func (k msgServer) AddToInTxTracker(goCtx context.Context, msg *types.MsgAddToInTxTracker) (*types.MsgAddToInTxTrackerResponse, error) {
	ctx := sdk.UnwrapSDKContext(goCtx)
	chain := k.zetaObserverKeeper.GetParams(ctx).GetChainFromChainID(msg.ChainId)
	if chain == nil {
		return nil, observertypes.ErrSupportedChains
	}

	adminPolicyAccount := k.zetaObserverKeeper.GetParams(ctx).GetAdminPolicyAccount(observertypes.Policy_Type_group1)
	isAdmin := msg.Creator == adminPolicyAccount
	isObserver := k.zetaObserverKeeper.IsAuthorized(ctx, msg.Creator, chain)

	isProven := false
	//@audit isObserver isAdmin does not verify proof if one is true
	if !(isAdmin || isObserver) && msg.Proof != nil {
		txBytes, err := k.VerifyProof(ctx, msg.Proof, msg.ChainId, msg.BlockHash, msg.TxIndex)
		if err != nil {
			return nil, types.ErrProofVerificationFail.Wrapf(err.Error())
		}

		if common.IsEVMChain(msg.ChainId) {
			err = k.VerifyEVMInTxBody(ctx, msg, txBytes)
			if err != nil {
				return nil, types.ErrTxBodyVerificationFail.Wrapf(err.Error())
			}
		} else {
			return nil, types.ErrTxBodyVerificationFail.Wrapf(fmt.Sprintf("cannot verify inTx body for chain %d", msg.ChainId))
		}
		isProven = true
	}

	// Sender needs to be either the admin policy account or an observer
	if !(isAdmin || isObserver || isProven) {
		return nil, errorsmod.Wrap(observertypes.ErrNotAuthorized, fmt.Sprintf("Creator %s", msg.Creator))
	}

    //@audit need to verify that InTxTracker has been executed

	k.SetInTxTracker(ctx, types.InTxTracker{
		ChainId:  msg.ChainId,
		TxHash:   msg.TxHash,
		CoinType: msg.CoinType,
	})
	return &types.MsgAddToInTxTrackerResponse{}, nil
}

Tools Used

vscode manual

Recommended Mitigation Steps

To verify that the InTxTracker has been executed

Assessed type

DoS