From e6bbc5b6f2f829f027317ce367cfce6a87284329 Mon Sep 17 00:00:00 2001 From: Edouard Schweisguth Date: Thu, 12 Jun 2025 15:24:45 +0200 Subject: [PATCH 1/4] Adds a Go Fuzz function for Parse() --- gostackparse_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/gostackparse_test.go b/gostackparse_test.go index 44ea69c..26262f2 100644 --- a/gostackparse_test.go +++ b/gostackparse_test.go @@ -7,6 +7,7 @@ package gostackparse import ( "bytes" + "embed" "encoding/json" "flag" "fmt" @@ -361,6 +362,49 @@ func BenchmarkGostackparse(b *testing.B) { b.ReportMetric(mbPerSec, "MiB/s") } +// Using go:embed to load test fixtures into memory, so the internal fuzzing infra doesn't need to handle individual files +// along with the fuzzer binary. +// +//go:embed test-fixtures/*.txt +var testFixtures embed.FS + +func FuzzParse(f *testing.F) { + files, err := testFixtures.ReadDir("test-fixtures") + require.NoError(f, err) + + for _, file := range files { + if strings.HasSuffix(file.Name(), ".txt") { + body, err := testFixtures.ReadFile(filepath.Join("test-fixtures", file.Name())) + require.NoError(f, err) + f.Add(body) + } + } + // Regression tests + // panic: runtime error: slice bounds out of range [28:26] + f.Add([]byte("goroutine 0 [0]:\n0()\n\t:0\n[originating from goroutine ")) + + f.Fuzz(func(t *testing.T, data []byte) { + gs, err := Parse(bytes.NewReader(data)) + if err != nil { + t.Skip() + } + // Invariant checks + for _, g := range gs { + for _, f := range g.Stack { + if f.Func == "" { + t.Errorf("func is empty: %+v", f) + } + } + } + }) +} + +// This is a regression test for a panic on a truncated line with an [originating from goroutine prefix. +func TestCrashRegression(t *testing.T) { + crashPayload := []byte("goroutine 0 [0]:\n0()\n\t:0\n[originating from goroutine ") + _, _ = Parse(bytes.NewReader(crashPayload)) +} + func TestFuzzCorupus(t *testing.T) { if os.Getenv("FUZZ_CORPUS") == "" { t.Skip("set FUZZ_CORPUS=true to generate fuzz corpus") From c3f07f06af26f7c2e44fe8396941d9b0cf0c9f2a Mon Sep 17 00:00:00 2001 From: Edouard Schweisguth Date: Thu, 12 Jun 2025 15:25:28 +0200 Subject: [PATCH 2/4] Fix crash on out of bound read caused by truncated line on "[originating from goroutine" line parsing. --- gostackparse.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gostackparse.go b/gostackparse.go index b5292cf..db41da3 100644 --- a/gostackparse.go +++ b/gostackparse.go @@ -83,7 +83,13 @@ func Parse(r io.Reader) ([]*Goroutine, []error) { goroutines = append(goroutines, g) } if state == stateOriginatingFrom { - ancestorIDStr := line[len(originatingFromPrefix) : len(line)-2] + // A truncated line that starts with "[originating from goroutine " but doesn't contain anything else afterwards + endOfIDStr := len(line) - 2 + if endOfIDStr < len(originatingFromPrefix) { + abortGoroutine("invalid originating from goroutine id") + continue + } + ancestorIDStr := line[len(originatingFromPrefix):endOfIDStr] ancestorID, err := strconv.Atoi(string(ancestorIDStr)) if err != nil { abortGoroutine(err.Error()) From 900913e2f31ee2f860e34d8e4f584d024e3cdc3a Mon Sep 17 00:00:00 2001 From: Edouard Schweisguth Date: Thu, 12 Jun 2025 16:46:14 +0200 Subject: [PATCH 3/4] Bump go version to at least 1.18 so it has go native fuzz support --- .github/workflows/go.yml | 22 +++++++++++----------- go.mod | 8 +++++++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 39abe26..910e428 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -2,23 +2,23 @@ name: Go on: push: - branches: [ main ] + branches: [main] pull_request: - branches: [ main ] + branches: [main] jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v2 - - name: Set up Go - uses: actions/setup-go@v2 - with: - go-version: 1.16 + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: 1.24 - - name: Build - run: go build -v ./... + - name: Build + run: go build -v ./... - - name: Test - run: go test -v ./... + - name: Test + run: go test -v ./... diff --git a/go.mod b/go.mod index b6dd635..a4e6fbe 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,11 @@ module github.com/DataDog/gostackparse -go 1.16 +go 1.18 require github.com/stretchr/testify v1.7.0 + +require ( + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect +) From 1815af6457ad2bee4977ccb11fe594e6b3f2cd4c Mon Sep 17 00:00:00 2001 From: Edouard Schweisguth Date: Fri, 13 Jun 2025 10:23:28 +0200 Subject: [PATCH 4/4] Use trim prefix & suffix to enforce correct ID parsing --- gostackparse.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gostackparse.go b/gostackparse.go index db41da3..1f19e79 100644 --- a/gostackparse.go +++ b/gostackparse.go @@ -83,13 +83,12 @@ func Parse(r io.Reader) ([]*Goroutine, []error) { goroutines = append(goroutines, g) } if state == stateOriginatingFrom { - // A truncated line that starts with "[originating from goroutine " but doesn't contain anything else afterwards - endOfIDStr := len(line) - 2 - if endOfIDStr < len(originatingFromPrefix) { - abortGoroutine("invalid originating from goroutine id") - continue - } - ancestorIDStr := line[len(originatingFromPrefix):endOfIDStr] + // Make sure we capture only the ID that we need. If it's truncated, + // it'll be handled by the error check below + ancestorIDStr := bytes.TrimSuffix( + bytes.TrimPrefix(line, originatingFromPrefix), + []byte("]:"), + ) ancestorID, err := strconv.Atoi(string(ancestorIDStr)) if err != nil { abortGoroutine(err.Error())