Skip to content

Commit 1f22d7d

Browse files
SungJin1212SungJin1212
committed
PRW2:Add bound checking for symbol referencs
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
1 parent ee2028a commit 1f22d7d

3 files changed

Lines changed: 158 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* [ENHANCEMENT] Compactor: Add concurrency for partition cleanup and mark block for deletion #7246
4646
* [ENHANCEMENT] Distributor: Validate metric name before removing empty labels. #7253
4747
* [ENHANCEMENT] Make cortex_ingester_tsdb_sample_ooo_delta metric per-tenant #7278
48+
* [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
4849
* [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
4950
* [BUGFIX] Ring: Change DynamoDB KV to retry indefinitely for WatchKey. #7088
5051
* [BUGFIX] Ruler: Add XFunctions validation support. #7111

‎pkg/util/push/push.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,10 @@ func convertV2RequestToV1(req *cortexpb.PreallocWriteRequestV2, enableTypeAndUni
217217
return v1Req, fmt.Errorf("TimeSeries must contain at least one sample or histogram for series %v", lbs.String())
218218
}
219219

220+
if int(v2Ts.Metadata.UnitRef) >= len(symbols) {
221+
return v1Req, fmt.Errorf("invalid UnitRef %d: exceeds symbols length %d for series %v", v2Ts.Metadata.UnitRef, len(symbols), lbs.String())
222+
}
223+
220224
unit := symbols[v2Ts.Metadata.UnitRef]
221225
metricType := v2Ts.Metadata.Type
222226
shouldAttachTypeAndUnitLabels := enableTypeAndUnitLabels && (metricType != cortexpb.METRIC_TYPE_UNSPECIFIED || unit != "")
@@ -253,7 +257,11 @@ func convertV2RequestToV1(req *cortexpb.PreallocWriteRequestV2, enableTypeAndUni
253257
if err != nil {
254258
return v1Req, err
255259
}
256-
v1Metadata = append(v1Metadata, convertV2ToV1Metadata(metricName, symbols, v2Ts.Metadata))
260+
metadata, err := convertV2ToV1Metadata(metricName, symbols, v2Ts.Metadata)
261+
if err != nil {
262+
return v1Req, err
263+
}
264+
v1Metadata = append(v1Metadata, metadata)
257265
}
258266
}
259267

@@ -267,7 +275,7 @@ func shouldConvertV2Metadata(metadata cortexpb.MetadataV2) bool {
267275
return !(metadata.HelpRef == 0 && metadata.UnitRef == 0 && metadata.Type == cortexpb.METRIC_TYPE_UNSPECIFIED) //nolint:staticcheck
268276
}
269277

270-
func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.MetadataV2) *cortexpb.MetricMetadata {
278+
func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.MetadataV2) (*cortexpb.MetricMetadata, error) {
271279
t := cortexpb.UNKNOWN
272280

273281
switch metadata.Type {
@@ -287,12 +295,19 @@ func convertV2ToV1Metadata(name string, symbols []string, metadata cortexpb.Meta
287295
t = cortexpb.STATESET
288296
}
289297

298+
if int(metadata.UnitRef) >= len(symbols) {
299+
return nil, fmt.Errorf("invalid UnitRef %d: exceeds symbols length %d", metadata.UnitRef, len(symbols))
300+
}
301+
if int(metadata.HelpRef) >= len(symbols) {
302+
return nil, fmt.Errorf("invalid HelpRef %d: exceeds symbols length %d", metadata.HelpRef, len(symbols))
303+
}
304+
290305
return &cortexpb.MetricMetadata{
291306
Type: t,
292307
MetricFamilyName: name,
293308
Unit: symbols[metadata.UnitRef],
294309
Help: symbols[metadata.HelpRef],
295-
}
310+
}, nil
296311
}
297312

298313
func convertV2ToV1Exemplars(b *labels.ScratchBuilder, symbols []string, v2Exemplars []cortexpb.ExemplarV2) ([]cortexpb.Exemplar, error) {

‎pkg/util/push/push_test.go‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,145 @@ func Test_convertV2RequestToV1(t *testing.T) {
450450
assert.Equal(t, 1, len(v1Req.Metadata))
451451
}
452452

453+
func Test_convertV2RequestToV1_InvalidSymbolRefs(t *testing.T) {
454+
symbols := []string{"", "__name__", "test_metric", "unit", "help"}
455+
456+
tests := []struct {
457+
name string
458+
v2Req *cortexpb.PreallocWriteRequestV2
459+
expectedError string
460+
}{
461+
{
462+
name: "invalid UnitRef out of bounds",
463+
v2Req: &cortexpb.PreallocWriteRequestV2{
464+
WriteRequestV2: cortexpb.WriteRequestV2{
465+
Symbols: symbols,
466+
Timeseries: []cortexpb.PreallocTimeseriesV2{
467+
{
468+
TimeSeriesV2: &cortexpb.TimeSeriesV2{
469+
LabelsRefs: []uint32{1, 2, 3, 4},
470+
Metadata: cortexpb.MetadataV2{
471+
Type: cortexpb.METRIC_TYPE_COUNTER,
472+
UnitRef: 1983,
473+
HelpRef: 0,
474+
},
475+
Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
476+
},
477+
},
478+
},
479+
},
480+
},
481+
expectedError: "invalid UnitRef 1983: exceeds symbols length 5",
482+
},
483+
{
484+
name: "invalid HelpRef out of bounds in metadata conversion",
485+
v2Req: &cortexpb.PreallocWriteRequestV2{
486+
WriteRequestV2: cortexpb.WriteRequestV2{
487+
Symbols: symbols,
488+
Timeseries: []cortexpb.PreallocTimeseriesV2{
489+
{
490+
TimeSeriesV2: &cortexpb.TimeSeriesV2{
491+
LabelsRefs: []uint32{1, 2, 3, 4},
492+
Metadata: cortexpb.MetadataV2{
493+
Type: cortexpb.METRIC_TYPE_GAUGE,
494+
UnitRef: 0,
495+
HelpRef: 9999,
496+
},
497+
Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
498+
},
499+
},
500+
},
501+
},
502+
},
503+
expectedError: "invalid HelpRef 9999: exceeds symbols length 5",
504+
},
505+
{
506+
name: "valid symbol refs should not error",
507+
v2Req: &cortexpb.PreallocWriteRequestV2{
508+
WriteRequestV2: cortexpb.WriteRequestV2{
509+
Symbols: symbols,
510+
Timeseries: []cortexpb.PreallocTimeseriesV2{
511+
{
512+
TimeSeriesV2: &cortexpb.TimeSeriesV2{
513+
LabelsRefs: []uint32{1, 2, 3, 4},
514+
Metadata: cortexpb.MetadataV2{
515+
Type: cortexpb.METRIC_TYPE_COUNTER,
516+
UnitRef: 3,
517+
HelpRef: 4,
518+
},
519+
Samples: []cortexpb.Sample{{Value: 1, TimestampMs: 1}},
520+
},
521+
},
522+
},
523+
},
524+
},
525+
expectedError: "",
526+
},
527+
}
528+
529+
for _, tt := range tests {
530+
t.Run(tt.name, func(t *testing.T) {
531+
_, err := convertV2RequestToV1(tt.v2Req, false)
532+
if tt.expectedError == "" {
533+
assert.NoError(t, err)
534+
} else {
535+
assert.Error(t, err)
536+
assert.Contains(t, err.Error(), tt.expectedError)
537+
}
538+
})
539+
}
540+
}
541+
542+
func Test_convertV2ToV1Metadata_InvalidSymbolRefs(t *testing.T) {
543+
symbols := []string{"", "__name__", "test_metric", "unit", "help"}
544+
545+
tests := []struct {
546+
name string
547+
metadata cortexpb.MetadataV2
548+
expectedError string
549+
}{
550+
{
551+
name: "invalid UnitRef",
552+
metadata: cortexpb.MetadataV2{
553+
Type: cortexpb.METRIC_TYPE_COUNTER,
554+
UnitRef: 100,
555+
HelpRef: 0,
556+
},
557+
expectedError: "invalid UnitRef 100: exceeds symbols length 5",
558+
},
559+
{
560+
name: "invalid HelpRef",
561+
metadata: cortexpb.MetadataV2{
562+
Type: cortexpb.METRIC_TYPE_GAUGE,
563+
UnitRef: 0,
564+
HelpRef: 200, // Out of bounds
565+
},
566+
expectedError: "invalid HelpRef 200: exceeds symbols length 5",
567+
},
568+
{
569+
name: "valid refs",
570+
metadata: cortexpb.MetadataV2{
571+
Type: cortexpb.METRIC_TYPE_COUNTER,
572+
UnitRef: 3,
573+
HelpRef: 4,
574+
},
575+
expectedError: "",
576+
},
577+
}
578+
579+
for _, tt := range tests {
580+
t.Run(tt.name, func(t *testing.T) {
581+
_, err := convertV2ToV1Metadata("test_metric", symbols, tt.metadata)
582+
if tt.expectedError == "" {
583+
assert.NoError(t, err)
584+
} else {
585+
assert.Error(t, err)
586+
assert.Contains(t, err.Error(), tt.expectedError)
587+
}
588+
})
589+
}
590+
}
591+
453592
func TestHandler_remoteWrite(t *testing.T) {
454593
var limits validation.Limits
455594
flagext.DefaultValues(&limits)

0 commit comments

Comments
 (0)