chore: prevent EOTS private key extraction via duplicate height validation (backport #738) (#745)

<hr>This is an automatic backport of pull request #738 done by
[Mergify](https://mergify.com).

Co-authored-by: Lazar <[email protected]>

Author: mergify[bot]

CHANGELOG.md

@@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * [#736](https://github.com/babylonlabs-io/finality-provider/pull/736) chore: disable flag on unsafe/test RPCs
 * [#737](https://github.com/babylonlabs-io/finality-provider/pull/737) chore: add fp identifier to logs
 * [#739](https://github.com/babylonlabs-io/finality-provider/pull/739) chore: bump babylon to `v4.0.0`
+* [#738](https://github.com/babylonlabs-io/finality-provider/pull/738) chore: prevent EOTS private key extraction via duplicate height validation
 
 ## v2.0.0-rc.5
 

clientcontroller/babylon/consumer.go

@@ -20,6 +20,7 @@ import (
 	"github.com/babylonlabs-io/finality-provider/clientcontroller/api"
 	fpcfg "github.com/babylonlabs-io/finality-provider/finality-provider/config"
 	"github.com/babylonlabs-io/finality-provider/types"
+	"github.com/babylonlabs-io/finality-provider/util"
 )
 
 var _ api.ConsumerController = &BabylonConsumerController{}
@@ -240,10 +241,18 @@ func (bc *BabylonConsumerController) queryLatestBlocks(startKey []byte, count ui
 		return nil, fmt.Errorf("failed to query finalized blocks: %w", err)
 	}
 
+	// Validate no duplicate heights from RPC response (defense-in-depth)
+	// Malicious/buggy RPC could return duplicate heights causing EOTS key extraction
+	heights := make([]uint64, 0, len(res.Blocks))
 	for _, b := range res.Blocks {
+		heights = append(heights, b.Height)
 		blocks = append(blocks, types.NewBlockInfo(b.Height, b.AppHash, b.Finalized))
 	}
 
+	if err := util.ValidateNoDuplicateHeights(heights); err != nil {
+		return nil, fmt.Errorf("RPC returned invalid block list: %w", err)
+	}
+
 	return blocks, nil
 }
 

eotsmanager/localmanager.go

@@ -313,6 +313,15 @@ func (lm *LocalEOTSManager) SignBatchEOTS(req *SignBatchEOTSRequest) ([]SignData
 
 	eotsPk, chainID := req.UID, req.ChainID
 
+	heights := make([]uint64, 0, len(req.SignRequest))
+	for _, request := range req.SignRequest {
+		heights = append(heights, request.Height)
+	}
+
+	if err := util.ValidateNoDuplicateHeights(heights); err != nil {
+		return nil, fmt.Errorf("%w: %w", eotstypes.ErrDuplicateHeight, err)
+	}
+
 	keyName, err := lm.es.GetEOTSKeyName(eotsPk)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get EOTS key name: %w", err)
@@ -323,12 +332,7 @@ func (lm *LocalEOTSManager) SignBatchEOTS(req *SignBatchEOTSRequest) ([]SignData
 		return nil, fmt.Errorf("failed to get EOTS private key: %w", err)
 	}
 
-	// Extract all heights for batch lookup
-	heights := make([]uint64, 0, len(req.SignRequest))
-	for _, request := range req.SignRequest {
-		heights = append(heights, request.Height)
-	}
-
+	// Use heights extracted above for validation
 	existingRecords, err := lm.es.GetSignRecordsBatch(eotsPk, chainID, heights)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get existing sign records: %w", err)

eotsmanager/service/rpcserver.go

@@ -14,6 +14,7 @@ import (
 	"github.com/babylonlabs-io/finality-provider/eotsmanager/config"
 	"github.com/babylonlabs-io/finality-provider/eotsmanager/proto"
 	"github.com/babylonlabs-io/finality-provider/eotsmanager/types"
+	"github.com/babylonlabs-io/finality-provider/util"
 )
 
 // rpcServer is the main RPC server for the EOTS daemon that handles
@@ -122,6 +123,17 @@ func (r *rpcServer) SignSchnorrSig(_ context.Context, req *proto.SignSchnorrSigR
 // SignBatchEOTS signs multiple EOTS in batch
 func (r *rpcServer) SignBatchEOTS(_ context.Context, req *proto.SignBatchEOTSRequest) (
 	*proto.SignBatchEOTSResponse, error) {
+	// Validate no duplicate heights before forwarding to manager (defense-in-depth)
+	heights := make([]uint64, len(req.SignRequests))
+	for i, signReq := range req.SignRequests {
+		heights[i] = signReq.Height
+	}
+
+	if err := util.ValidateNoDuplicateHeights(heights); err != nil {
+		return nil, status.Error(codes.InvalidArgument, //nolint:wrapcheck
+			fmt.Sprintf("duplicate height in batch: %v", err))
+	}
+
 	signRequests := make([]*eotsmanager.SignDataRequest, len(req.SignRequests))
 	for i, signReq := range req.SignRequests {
 		signRequests[i] = &eotsmanager.SignDataRequest{

eotsmanager/types/errors.go

@@ -6,4 +6,5 @@ import "errors"
 var (
 	ErrFinalityProviderAlreadyExisted = errors.New("the finality provider has already existed")
 	ErrDoubleSign                     = errors.New("double sign")
+	ErrDuplicateHeight                = errors.New("duplicate height in batch request")
 )

util/validation.go

@@ -0,0 +1,33 @@
+//nolint:revive
+package util
+
+import "fmt"
+
+// HasDuplicateHeights checks if the provided slice of heights contains any duplicates.
+// Returns (true, duplicateHeight) if a duplicate is found, (false, 0) otherwise.
+// Uses a map for O(n) time complexity.
+//
+// This function is critical for preventing EOTS private key extraction attacks.
+// Signing the same height twice with different messages allows an attacker to
+// mathematically extract the private key through EOTS vulnerability.
+func HasDuplicateHeights(heights []uint64) (bool, uint64) {
+	seen := make(map[uint64]struct{}, len(heights))
+	for _, height := range heights {
+		if _, exists := seen[height]; exists {
+			return true, height
+		}
+		seen[height] = struct{}{}
+	}
+
+	return false, 0
+}
+
+// ValidateNoDuplicateHeights returns an error if duplicate heights are found in the slice.
+// This validation is essential for preventing EOTS key extraction via duplicate signing.
+func ValidateNoDuplicateHeights(heights []uint64) error {
+	if hasDup, dupHeight := HasDuplicateHeights(heights); hasDup {
+		return fmt.Errorf("duplicate height detected: %d", dupHeight)
+	}
+
+	return nil
+}

util/validation_test.go

@@ -0,0 +1,141 @@
+//nolint:revive
+package util_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/babylonlabs-io/finality-provider/util"
+)
+
+func TestHasDuplicateHeights(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name      string
+		heights   []uint64
+		expectDup bool
+		dupHeight uint64
+	}{
+		{
+			name:      "no duplicates",
+			heights:   []uint64{1, 2, 3, 4, 5},
+			expectDup: false,
+			dupHeight: 0,
+		},
+		{
+			name:      "duplicate at start",
+			heights:   []uint64{1, 1, 3},
+			expectDup: true,
+			dupHeight: 1,
+		},
+		{
+			name:      "duplicate in middle",
+			heights:   []uint64{1, 2, 2, 3},
+			expectDup: true,
+			dupHeight: 2,
+		},
+		{
+			name:      "duplicate at end",
+			heights:   []uint64{1, 2, 3, 3},
+			expectDup: true,
+			dupHeight: 3,
+		},
+		{
+			name:      "empty slice",
+			heights:   []uint64{},
+			expectDup: false,
+			dupHeight: 0,
+		},
+		{
+			name:      "single element",
+			heights:   []uint64{42},
+			expectDup: false,
+			dupHeight: 0,
+		},
+		{
+			name:      "two identical elements",
+			heights:   []uint64{100, 100},
+			expectDup: true,
+			dupHeight: 100,
+		},
+		{
+			name:      "non-consecutive unique heights",
+			heights:   []uint64{10, 20, 30, 15, 25},
+			expectDup: false,
+			dupHeight: 0,
+		},
+		{
+			name:      "large numbers no duplicates",
+			heights:   []uint64{1000000, 2000000, 3000000},
+			expectDup: false,
+			dupHeight: 0,
+		},
+		{
+			name:      "large numbers with duplicate",
+			heights:   []uint64{1000000, 2000000, 1000000},
+			expectDup: true,
+			dupHeight: 1000000,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			hasDup, dupHeight := util.HasDuplicateHeights(tt.heights)
+			require.Equal(t, tt.expectDup, hasDup)
+			if tt.expectDup {
+				require.Equal(t, tt.dupHeight, dupHeight)
+			} else {
+				require.Equal(t, uint64(0), dupHeight)
+			}
+		})
+	}
+}
+
+func TestValidateNoDuplicateHeights(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name      string
+		heights   []uint64
+		expectErr bool
+	}{
+		{
+			name:      "valid unique heights",
+			heights:   []uint64{1, 2, 3, 4, 5},
+			expectErr: false,
+		},
+		{
+			name:      "invalid with duplicates",
+			heights:   []uint64{1, 2, 2, 3},
+			expectErr: true,
+		},
+		{
+			name:      "empty is valid",
+			heights:   []uint64{},
+			expectErr: false,
+		},
+		{
+			name:      "single element is valid",
+			heights:   []uint64{1},
+			expectErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			err := util.ValidateNoDuplicateHeights(tt.heights)
+			if tt.expectErr {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), "duplicate height detected")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}