Skip to content

Commit

Permalink
fix(distributor): validate string table accesses
Browse files Browse the repository at this point in the history
  • Loading branch information
korniltsev committed Jan 6, 2025
1 parent 848179a commit d3fc4d2
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 0 deletions.
67 changes: 67 additions & 0 deletions pkg/test/integration/ingest_pprof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions pkg/validation/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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 != ""
}
Expand Down

0 comments on commit d3fc4d2

Please sign in to comment.