diff --git a/CHANGELOG.md b/CHANGELOG.md index f7c526927d..8c3445e315 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/util/push/push.go b/pkg/util/push/push.go index 4608f2c4ad..c0a3f1efc9 100644 --- a/pkg/util/push/push.go +++ b/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) { diff --git a/pkg/util/push/push_test.go b/pkg/util/push/push_test.go index ac7c02a874..ec7dec365a 100644 --- a/pkg/util/push/push_test.go +++ b/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)