[Bug]: De-dup Vote Extensions in ValidateVoteExtensions

[davidterpay]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

The main ValidateVoteExtensions utility function currently does not de-duplicate vote extensions included in the ExtendedCommitInfo. The code can be referenced here:

cosmos-sdk/baseapp/abci_utils.go

Lines 42 to 137 in ae19acc

	// ValidateVoteExtensions defines a helper function for verifying vote extension
	// signatures that may be passed or manually injected into a block proposal from
	// a proposer in PrepareProposal. It returns an error if any signature is invalid
	// or if unexpected vote extensions and/or signatures are found or less than 2/3
	// power is received.
	func ValidateVoteExtensions(
	ctx sdk.Context,
	valStore ValidatorStore,
	currentHeight int64,
	chainID string,
	extCommit abci.ExtendedCommitInfo,
	) error {
	cp := ctx.ConsensusParams()
	// Start checking vote extensions only **after** the vote extensions enable
	// height, because when `currentHeight == VoteExtensionsEnableHeight`
	// PrepareProposal doesn't get any vote extensions in its request.
	extsEnabled := cp.Abci != nil && currentHeight > cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0
	marshalDelimitedFn := func(msg proto.Message) ([]byte, error) {
	var buf bytes.Buffer
	if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil {
	return nil, err
	}
	return buf.Bytes(), nil
	}
	var (
	// Total voting power of all vote extensions.
	totalVP int64
	// Total voting power of all validators that submitted valid vote extensions.
	sumVP int64
	)
	for _, vote := range extCommit.Votes {
	totalVP += vote.Validator.Power
	// Only check + include power if the vote is a commit vote. There must be super-majority, otherwise the
	// previous block (the block vote is for) could not have been committed.
	if vote.BlockIdFlag != cmtproto.BlockIDFlagCommit {
	continue
	}
	if !extsEnabled {
	if len(vote.VoteExtension) > 0 {
	return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight)
	}
	if len(vote.ExtensionSignature) > 0 {
	return fmt.Errorf("vote extensions disabled; received non-empty vote extension signature at height %d", currentHeight)
	}
	continue
	}
	if len(vote.ExtensionSignature) == 0 {
	return fmt.Errorf("vote extensions enabled; received empty vote extension signature at height %d", currentHeight)
	}
	valConsAddr := sdk.ConsAddress(vote.Validator.Address)
	pubKeyProto, err := valStore.GetPubKeyByConsAddr(ctx, valConsAddr)
	if err != nil {
	return fmt.Errorf("failed to get validator %X public key: %w", valConsAddr, err)
	}
	cmtPubKey, err := cryptoenc.PubKeyFromProto(pubKeyProto)
	if err != nil {
	return fmt.Errorf("failed to convert validator %X public key: %w", valConsAddr, err)
	}
	cve := cmtproto.CanonicalVoteExtension{
	Extension: vote.VoteExtension,
	Height: currentHeight - 1, // the vote extension was signed in the previous height
	Round: int64(extCommit.Round),
	ChainId: chainID,
	}
	extSignBytes, err := marshalDelimitedFn(&cve)
	if err != nil {
	return fmt.Errorf("failed to encode CanonicalVoteExtension: %w", err)
	}
	if !cmtPubKey.VerifySignature(extSignBytes, vote.ExtensionSignature) {
	return fmt.Errorf("failed to verify validator %X vote extension signature", valConsAddr)
	}
	sumVP += vote.Validator.Power
	}
	if totalVP > 0 {
	percentSubmitted := math.LegacyNewDecFromInt(math.NewInt(sumVP)).Quo(math.LegacyNewDecFromInt(math.NewInt(totalVP)))
	if percentSubmitted.LT(VoteExtensionThreshold) {
	return fmt.Errorf("insufficient cumulative voting power received to verify vote extensions; got: %s, expected: >=%s", percentSubmitted, VoteExtensionThreshold)
	}
	}
	return nil
	}

The fix here is simple and I will reference the PR once it is made. This can allow malicious proposers to effectively bypass the 2/3+ quorum required by default to accept a block with vote extensions.

Cosmos SDK Version

main

How to reproduce?

Something like the following can happen:

1. proposer retrieves the extended commit info from the previous height.
2. only includes their own vote extension, duplicated with enough voting power to reach quorum
3. includes that in the current block
4. ValidateVoteExtensions accepts the block proposal and proposer can manipulate on-chain state depending on the context of how vote extensions are utilized (oracles, random numbers, etc.)

[davidterpay]

Working PR: #18895