Description

The SetNodeKeys message allows a node to supply a public key that will be used for the TSS signing:

func (k msgServer) SetNodeKeys(goCtx context.Context, msg *types.MsgSetNodeKeys) (*types.MsgSetNodeKeysResponse, error) {
	ctx := sdk.UnwrapSDKContext(goCtx)
	addr, err := sdk.AccAddressFromBech32(msg.Creator)
	if err != nil {
		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("msg creator %s not valid", msg.Creator))
	}
	_, found := k.GetNodeAccount(ctx, msg.Creator)
	if !found {
		na := types.NodeAccount{
			Creator:     msg.Creator,
			Index:       msg.Creator,
			NodeAddress: addr,
			PubkeySet:   msg.PubkeySet,
			NodeStatus:  types.NodeStatus_Unknown,
		}
		k.SetNodeAccount(ctx, na)
	} else {
		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("msg creator %s already has a node account", msg.Creator))
	}

	return &types.MsgSetNodeKeysResponse{}, nil
}

The issue is that there are no authentication or verification checks in place to limit who can call it. As a result, anyone can call the function and add their public key to the list.

Impact

The list of node accounts is fetched in the InitializeGenesisKeygen and in the zeta client's genNewKeysAtBlock in order to determine the public keys that should be used for the TSS signing. If anyone is able to add their public key before the list is queried (for example, just before the block number that the new keys will be generated), they could potentially be able to control enough signatures to pass the threshold and sign transactions or otherwise create a denial of service where the TSS can no longer sign anything.

Recommendations

Adding node accounts should be a privileged operation, and only trusted keys should be able to be added.

Remediation

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