PRW2:Add bound checking for symbol referencs

CHANGELOG.md

@@ -46,6 +46,7 @@
 * [ENHANCEMENT] Distributor: Validate metric name before removing empty labels. #7253
 * [ENHANCEMENT] Make cortex_ingester_tsdb_sample_ooo_delta metric per-tenant #7278
 * [ENHANCEMENT] Distributor: Add dimension `nhcb` to keep track of nhcb samples in `cortex_distributor_received_samples_total` and `cortex_distributor_samples_in_total` metrics.
+* [BUGFIX] Distributor: Add bounds checking for symbol references in Remote Write V2 requests to prevent panics when UnitRef or HelpRef exceed the symbols array length. #7289
 * [BUGFIX] Distributor: If remote write v2 is disabled, explicitly return HTTP 415 (Unsupported Media Type) for Remote Write V2 requests instead of attempting to parse them as V1. #7238
 * [BUGFIX] Ring: Change DynamoDB KV to retry indefinitely for WatchKey. #7088
 * [BUGFIX] Ruler: Add XFunctions validation support. #7111

pkg/util/push/push.go

@@ -217,6 +217,10 @@ func convertV2RequestToV1(req *cortexpb.PreallocWriteRequestV2, enableTypeAndUni
 			return v1Req, fmt.Errorf("TimeSeries must contain at least one sample or histogram for series %v", lbs.String())
 		}
 
+		if int(v2Ts.Metadata.UnitRef) >= len(symbols) {
+			return v1Req, fmt.Errorf("invalid UnitRef %d: exceeds symbols length %d", v2Ts.Metadata.UnitRef, len(symbols))
+		}
+
 		unit := symbols[v2Ts.Metadata.UnitRef]
 		metricType := v2Ts.Metadata.Type
 		shouldAttachTypeAndUnitLabels := enableTypeAndUnitLabels && (metricType != cortexpb.METRIC_TYPE_UNSPECIFIED || unit != "")
@@ -253,7 +257,11 @@ func convertV2RequestToV1(req *cortexpb.PreallocWriteRequestV2, enableTypeAndUni
 			if err != nil {
 				return v1Req, err
 			}
-			v1Metadata = append(v1Metadata, convertV2ToV1Metadata(metricName, symbols, v2Ts.Metadata))
+			metadata, err := convertV2ToV1Metadata(metricName, symbols, v2Ts.Metadata)
+			if err != nil {
+				return v1Req, err
+			}
+			v1Metadata = append(v1Metadata, metadata)
 		}
 	}
 
@@ -267,7 +275,7 @@ func shouldConvertV2Metadata(metadata cortexpb.MetadataV2) bool {
 	return !(metadata.HelpRef == 0 && metadata.UnitRef == 0 && metadata.Type == cortexpb.METRIC_TYPE_UNSPECIFIED) //nolint:staticcheck
 }
 
-func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.MetadataV2) *cortexpb.MetricMetadata {
+func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.MetadataV2) (*cortexpb.MetricMetadata, error) {
 	t := cortexpb.UNKNOWN
 
 	switch metadata.Type {
@@ -287,12 +295,19 @@ func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.Meta
 		t = cortexpb.STATESET
 	}
 
+	if int(metadata.UnitRef) >= len(symbols) {
+		return nil, fmt.Errorf("invalid UnitRef %d: exceeds symbols length %d", metadata.UnitRef, len(symbols))
+	}
+	if int(metadata.HelpRef) >= len(symbols) {
+		return nil, fmt.Errorf("invalid HelpRef %d: exceeds symbols length %d", metadata.HelpRef, len(symbols))
+	}
+
 	return &cortexpb.MetricMetadata{
 		Type:             t,
 		MetricFamilyName: name,
 		Unit:             symbols[metadata.UnitRef],
 		Help:             symbols[metadata.HelpRef],
-	}
+	}, nil
 }
 
 func convertV2ToV1Exemplars(b *labels.ScratchBuilder, symbols []string, v2Exemplars []cortexpb.ExemplarV2) ([]cortexpb.Exemplar, error) {

pkg/util/push/push_test.go

@@ -450,6 +450,145 @@ func Test_convertV2RequestToV1(t *testing.T) {
 	assert.Equal(t, 1, len(v1Req.Metadata))
 }
 
+func Test_convertV2RequestToV1_InvalidSymbolRefs(t *testing.T) {
+	symbols := []string{"", "__name__", "test_metric", "unit", "help"}
+
+	tests := []struct {
+		name          string
+		v2Req         *cortexpb.PreallocWriteRequestV2
+		expectedError string
+	}{
+		{
+			name: "invalid UnitRef out of bounds",
+			v2Req: &cortexpb.PreallocWriteRequestV2{
+				WriteRequestV2: cortexpb.WriteRequestV2{
+					Symbols: symbols,
+					Timeseries: []cortexpb.PreallocTimeseriesV2{
+						{
+							TimeSeriesV2: &cortexpb.TimeSeriesV2{
+								LabelsRefs: []uint32{1, 2, 3, 4},
+								Metadata: cortexpb.MetadataV2{
+									Type:    cortexpb.METRIC_TYPE_COUNTER,
+									UnitRef: 1983,
+									HelpRef: 0,
+								},
+								Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
+							},
+						},
+					},
+				},
+			},
+			expectedError: "invalid UnitRef 1983: exceeds symbols length 5",
+		},
+		{
+			name: "invalid HelpRef out of bounds in metadata conversion",
+			v2Req: &cortexpb.PreallocWriteRequestV2{
+				WriteRequestV2: cortexpb.WriteRequestV2{
+					Symbols: symbols,
+					Timeseries: []cortexpb.PreallocTimeseriesV2{
+						{
+							TimeSeriesV2: &cortexpb.TimeSeriesV2{
+								LabelsRefs: []uint32{1, 2, 3, 4},
+								Metadata: cortexpb.MetadataV2{
+									Type:    cortexpb.METRIC_TYPE_GAUGE,
+									UnitRef: 0,
+									HelpRef: 9999,
+								},
+								Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
+							},
+						},
+					},
+				},
+			},
+			expectedError: "invalid HelpRef 9999: exceeds symbols length 5",
+		},
+		{
+			name: "valid symbol refs should not error",
+			v2Req: &cortexpb.PreallocWriteRequestV2{
+				WriteRequestV2: cortexpb.WriteRequestV2{
+					Symbols: symbols,
+					Timeseries: []cortexpb.PreallocTimeseriesV2{
+						{
+							TimeSeriesV2: &cortexpb.TimeSeriesV2{
+								LabelsRefs: []uint32{1, 2, 3, 4},
+								Metadata: cortexpb.MetadataV2{
+									Type:    cortexpb.METRIC_TYPE_COUNTER,
+									UnitRef: 3,
+									HelpRef: 4,
+								},
+								Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
+							},
+						},
+					},
+				},
+			},
+			expectedError: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := convertV2RequestToV1(tt.v2Req, false)
+			if tt.expectedError == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), tt.expectedError)
+			}
+		})
+	}
+}
+
+func Test_convertV2ToV1Metadata_InvalidSymbolRefs(t *testing.T) {
+	symbols := []string{"", "__name__", "test_metric", "unit", "help"}
+
+	tests := []struct {
+		name          string
+		metadata      cortexpb.MetadataV2
+		expectedError string
+	}{
+		{
+			name: "invalid UnitRef",
+			metadata: cortexpb.MetadataV2{
+				Type:    cortexpb.METRIC_TYPE_COUNTER,
+				UnitRef: 100,
+				HelpRef: 0,
+			},
+			expectedError: "invalid UnitRef 100: exceeds symbols length 5",
+		},
+		{
+			name: "invalid HelpRef",
+			metadata: cortexpb.MetadataV2{
+				Type:    cortexpb.METRIC_TYPE_GAUGE,
+				UnitRef: 0,
+				HelpRef: 200, // Out of bounds
+			},
+			expectedError: "invalid HelpRef 200: exceeds symbols length 5",
+		},
+		{
+			name: "valid refs",
+			metadata: cortexpb.MetadataV2{
+				Type:    cortexpb.METRIC_TYPE_COUNTER,
+				UnitRef: 3,
+				HelpRef: 4,
+			},
+			expectedError: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := convertV2ToV1Metadata("test_metric", symbols, tt.metadata)
+			if tt.expectedError == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), tt.expectedError)
+			}
+		})
+	}
+}
+
 func TestHandler_remoteWrite(t *testing.T) {
 	var limits validation.Limits
 	flagext.DefaultValues(&limits)