diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index 22a4e75dae..98a1cefc2a 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -1122,6 +1122,7 @@ func TestPush_ShuffleSharding(t *testing.T) { LocationId: []uint64{1}, Value: []int64{1}, }}, + StringTable: []string{""}, }).WriteTo(&buf) require.NoError(t, err) profileBytes := buf.Bytes() diff --git a/pkg/test/integration/ingest_pprof_test.go b/pkg/test/integration/ingest_pprof_test.go index 1ffe6b125a..d2eb9d9968 100644 --- a/pkg/test/integration/ingest_pprof_test.go +++ b/pkg/test/integration/ingest_pprof_test.go @@ -352,6 +352,73 @@ func TestGodeltaprofRelabelPush(t *testing.T) { assert.Equal(t, expected, actual) } +func TestPushStringTableOOBSampleType(t *testing.T) { + const blockSize = 1024 + const metric = "godeltaprof_memory" + + p := PyroscopeTest{} + p.Start(t) + defer p.Stop(t) + + testdata := []struct { + name string + corrupt func(p *testhelper.ProfileBuilder) + expectedErr string + }{ + { + name: "sample type", + corrupt: func(p *testhelper.ProfileBuilder) { + p.SampleType[0].Type = 100500 + }, + expectedErr: "sample type type string index out of range", + }, + { + name: "function name", + corrupt: func(p *testhelper.ProfileBuilder) { + p.Function[0].Name = 100500 + }, + expectedErr: "function name string index out of range", + }, + { + name: "mapping", + corrupt: func(p *testhelper.ProfileBuilder) { + p.Mapping[0].Filename = 100500 + }, + expectedErr: "mapping file name string index out of range", + }, + { + name: "Sample label", + corrupt: func(p *testhelper.ProfileBuilder) { + p.Sample[0].Label = []*profilev1.Label{{ + Key: 100500, + }} + }, + expectedErr: "sample label string index out of range", + }, + { + name: "String 0 not empty", + corrupt: func(p *testhelper.ProfileBuilder) { + p.StringTable[0] = "hmmm" + }, + expectedErr: "string 0 should be empty string", + }, + } + for _, td := range testdata { + t.Run(td.name, func(t *testing.T) { + p1 := testhelper.NewProfileBuilder(time.Now().Add(-time.Second).UnixNano()). + MemoryProfile(). + ForStacktraceString("my", "other"). + AddSamples(239, 239*blockSize, 1000, 1000*blockSize) + td.corrupt(p1) + p1bs, err := p1.Profile.MarshalVT() + require.NoError(t, err) + + rb := p.NewRequestBuilder(t) + rb.Push(rb.PushPPROFRequestFromBytes(p1bs, metric), 400, td.expectedErr) + }) + } +} + func TestPush(t *testing.T) { p := new(PyroscopeTest) p.Start(t) diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go index e17a36f7d2..c56de80b5d 100644 --- a/pkg/validation/validate.go +++ b/pkg/validation/validate.go @@ -274,6 +274,10 @@ func ValidateProfile(limits ProfileValidationLimits, tenantID string, prof *goog return NewErrorf(MalformedProfile, "function id is 0") } } + + if err := validateStringTableAccess(prof); err != nil { + return err + } for _, valueType := range prof.SampleType { stt := prof.StringTable[valueType.Type] if strings.Contains(stt, "-") { @@ -281,6 +285,7 @@ func ValidateProfile(limits ProfileValidationLimits, tenantID string, prof *goog } // todo check if sample type is valid from the promql parser perspective } + for _, s := range prof.StringTable { if !utf8.ValidString(s) { return NewErrorf(MalformedProfile, "invalid utf8 string hex: %s", hex.EncodeToString([]byte(s))) @@ -289,6 +294,44 @@ func ValidateProfile(limits ProfileValidationLimits, tenantID string, prof *goog return nil } +func validateStringTableAccess(prof *googlev1.Profile) error { + if len(prof.StringTable) == 0 || prof.StringTable[0] != "" { + return NewErrorf(MalformedProfile, "string 0 should be empty string") + } + for _, valueType := range prof.SampleType { + if int(valueType.Type) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "sample type type string index out of range") + } + } + for _, sample := range prof.Sample { + for _, lbl := range sample.Label { + if int(lbl.Str) >= len(prof.StringTable) || int(lbl.Key) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "sample label string index out of range") + } + } + } + for _, function := range prof.Function { + if int(function.Name) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "function name string index out of range") + } + if int(function.SystemName) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "function system name index string out of range") + } + if int(function.Filename) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "function file name string index out of range") + } + } + for _, mapping := range prof.Mapping { + if int(mapping.Filename) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "mapping file name string index out of range") + } + if int(mapping.BuildId) >= len(prof.StringTable) { + return NewErrorf(MalformedProfile, "mapping build id string index out of range") + } + } + return nil +} + func isValidServiceName(serviceNameValue string) bool { return serviceNameValue != "" } diff --git a/pkg/validation/validate_test.go b/pkg/validation/validate_test.go index 696b8f9a0e..c6d9719309 100644 --- a/pkg/validation/validate_test.go +++ b/pkg/validation/validate_test.go @@ -297,7 +297,7 @@ func TestValidateProfile(t *testing.T) { { "truncate labels and stacktrace", &googlev1.Profile{ - StringTable: []string{"foo", "/foo/bar"}, + StringTable: []string{"", "foo", "/foo/bar"}, Sample: []*googlev1.Sample{ { LocationId: []uint64{0, 1, 2, 3, 4, 5}, @@ -312,7 +312,7 @@ func TestValidateProfile(t *testing.T) { nil, func(t *testing.T, profile *googlev1.Profile) { t.Helper() - require.Equal(t, []string{"foo", "bar"}, profile.StringTable) + require.Equal(t, []string{"", "foo", "bar"}, profile.StringTable) require.Equal(t, []uint64{4, 5}, profile.Sample[0].LocationId) }, }, @@ -345,7 +345,8 @@ func TestValidateProfile(t *testing.T) { { name: "just in the ingestion window", profile: &googlev1.Profile{ - TimeNanos: now.Add(-1 * time.Minute).UnixNano(), + TimeNanos: now.Add(-1 * time.Minute).UnixNano(), + StringTable: []string{""}, }, limits: MockLimits{ RejectOlderThanValue: time.Hour, @@ -353,8 +354,10 @@ func TestValidateProfile(t *testing.T) { }, }, { - name: "without timestamp", - profile: &googlev1.Profile{}, + name: "without timestamp", + profile: &googlev1.Profile{ + StringTable: []string{""}, + }, limits: MockLimits{ RejectOlderThanValue: time.Hour, RejectNewerThanValue: 10 * time.Minute,