fix(api): bound and dedupe blob sidecar indices

.github/workflows/golangci-lint.yaml

@@ -9,7 +9,7 @@ on:
 permissions:
   contents: read
   # Optional: allow read access to pull request. Use with `only-new-issues` option.
-  # pull-requests: read
+  pull-requests: read
 jobs:
   golangci:
     name: lint
@@ -32,7 +32,7 @@ jobs:
           # args: --issues-exit-code=0
 
           # Optional: show only new issues if it's a pull request. The default value is `false`.
-          # only-new-issues: true
+          only-new-issues: true
 
           # Optional: if set to true then the all caching functionality will be complete disabled,
           #           takes precedence over all other caching options.

pkg/api/handler.go

@@ -18,6 +18,11 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// maxBlobSidecarIndices bounds how many `indices` query parameters a single
+// blob sidecars request may supply. Far above any fork's maximum blobs per
+// block, while preventing arbitrarily large requests.
+const maxBlobSidecarIndices = 1024
+
 // Handler is an API handler that is responsible for negotiating with a HTTP api.
 // All http-level concerns should be handled in this package, with the "namespaces" (eth/checkpointz)
 // handling all business logic and dealing with concrete types.
@@ -582,18 +587,9 @@ func (h *Handler) handleEthV1BeaconBlobSidecars(ctx context.Context, r *http.Req
 		return NewBadRequestResponse(nil), err
 	}
 
-	queryParams := r.URL.Query()
-	indicesRaw := queryParams["indices"]
-
-	indices := make([]int, 0, len(indicesRaw))
-
-	for _, index := range indicesRaw {
-		converted, errr := strconv.Atoi(index)
-		if errr != nil {
-			return NewBadRequestResponse(nil), errr
-		}
-
-		indices = append(indices, converted)
+	indices, err := parseBlobSidecarIndices(r.URL.Query()["indices"])
+	if err != nil {
+		return NewBadRequestResponse(nil), err
 	}
 
 	sidecars, dataVersion, err := h.eth.BlobSidecars(ctx, id, indices)
@@ -622,3 +618,36 @@ func (h *Handler) handleEthV1BeaconBlobSidecars(ctx context.Context, r *http.Req
 
 	return rsp, nil
 }
+
+// parseBlobSidecarIndices parses the `indices` query parameters as a bounded
+// set: values must be non-negative integers, duplicates are collapsed, and the
+// total number of supplied values is capped.
+func parseBlobSidecarIndices(raw []string) ([]int, error) {
+	if len(raw) > maxBlobSidecarIndices {
+		return nil, fmt.Errorf("too many indices: %d (max %d)", len(raw), maxBlobSidecarIndices)
+	}
+
+	seen := make(map[int]struct{}, len(raw))
+	indices := make([]int, 0, len(raw))
+
+	for _, value := range raw {
+		converted, err := strconv.Atoi(value)
+		if err != nil {
+			return nil, err
+		}
+
+		if converted < 0 {
+			return nil, fmt.Errorf("invalid index: %d", converted)
+		}
+
+		if _, exists := seen[converted]; exists {
+			continue
+		}
+
+		seen[converted] = struct{}{}
+
+		indices = append(indices, converted)
+	}
+
+	return indices, nil
+}

pkg/api/handler_internal_test.go

@@ -0,0 +1,67 @@
+package api
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseBlobSidecarIndices(t *testing.T) {
+	tests := []struct {
+		name     string
+		raw      []string
+		expected []int
+		wantErr  bool
+	}{
+		{"empty", []string{}, []int{}, false},
+		{"single", []string{"0"}, []int{0}, false},
+		{"multiple", []string{"0", "1", "2"}, []int{0, 1, 2}, false},
+		{"duplicates collapsed", []string{"0", "0", "0", "1"}, []int{0, 1}, false},
+		{"order preserved", []string{"2", "0", "1"}, []int{2, 0, 1}, false},
+		{"negative rejected", []string{"-1"}, nil, true},
+		{"non-numeric rejected", []string{"abc"}, nil, true},
+		{"empty string rejected", []string{""}, nil, true},
+		{"at cap", manyIndices(maxBlobSidecarIndices), nil, false},
+		{"over cap rejected", manyIndices(maxBlobSidecarIndices + 1), nil, true},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			indices, err := parseBlobSidecarIndices(test.raw)
+
+			if test.wantErr {
+				require.Error(t, err)
+
+				return
+			}
+
+			require.NoError(t, err)
+
+			if test.expected != nil {
+				assert.Equal(t, test.expected, indices)
+			}
+		})
+	}
+}
+
+func TestParseBlobSidecarIndicesDuplicatesDoNotAmplify(t *testing.T) {
+	raw := make([]string, 0, maxBlobSidecarIndices)
+	for i := 0; i < maxBlobSidecarIndices; i++ {
+		raw = append(raw, "0")
+	}
+
+	indices, err := parseBlobSidecarIndices(raw)
+	require.NoError(t, err)
+	assert.Equal(t, []int{0}, indices)
+}
+
+func manyIndices(n int) []string {
+	raw := make([]string, 0, n)
+	for i := 0; i < n; i++ {
+		raw = append(raw, fmt.Sprintf("%d", i))
+	}
+
+	return raw
+}