fix(distributor): validate string table accesses

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()

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)

pkg/validation/validate.go

@@ -274,13 +274,18 @@ 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, "-") {
 			return NewErrorf(MalformedProfile, "sample type contains -")
 		}
 		// 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 != ""
 }

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,16 +345,19 @@ 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,
 				RejectNewerThanValue: 10 * time.Minute,
 			},
 		},
 		{
-			name:    "without timestamp",
-			profile: &googlev1.Profile{},
+			name: "without timestamp",
+			profile: &googlev1.Profile{
+				StringTable: []string{""},
+			},
 			limits: MockLimits{
 				RejectOlderThanValue: time.Hour,
 				RejectNewerThanValue: 10 * time.Minute,