From 39173892081a8614f56d165c34e7058db5a4c8e1 Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Wed, 18 Sep 2024 14:51:04 +0000 Subject: [PATCH] internal/scan: reorganize trace text layout in trace mode As paths are relative, it is not immediately evident to what module symbols belong to in traces mode. We hence reorganize things to make that explicit while avoiding clutter. Fixes golang/go#69490 Change-Id: Ic43e22954cbe3ff0ac458f75ee3a07706295fb5d Reviewed-on: https://go-review.googlesource.com/c/vuln/+/614135 Reviewed-by: Maceo Thompson LUCI-TryBot-Result: Go LUCI --- .../testfiles/source-call/source_call_text.ct | 16 +++++----- .../source-call/source_multientry_text.ct | 14 ++++----- .../source-call/source_subdir_text.ct | 16 +++++----- cmd/govulncheck/testdata/stdlib/config.json | 4 +-- .../testfiles/stdlib/source_stdlib_text.ct | 10 +++---- .../testdata/strip/testfiles/binary/strip.ct | 30 +++++++++---------- internal/scan/result_test.go | 2 +- internal/scan/template.go | 20 +++++++++---- internal/scan/testdata/source_traces.txt | 4 +-- internal/scan/text.go | 12 ++++++-- 10 files changed, 73 insertions(+), 55 deletions(-) diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct index 7ad1c52d..f90cb3fb 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_text.ct @@ -44,8 +44,8 @@ Vulnerability #1: GO-2021-0265 Fixed in: github.com/tidwall/gjson@v1.9.3 Example traces found: #1: for function github.com/tidwall/gjson.Result.Get - vuln.go:14:20: golang.org/vuln.main - gjson.go:296:17: github.com/tidwall/gjson.Result.Get + main @ golang.org/vuln/vuln.go:14:20 + Result.Get @ github.com/tidwall/gjson/gjson.go:296:17 Vulnerability #2: GO-2021-0054 Due to improper bounds checking, maliciously crafted JSON objects can cause @@ -57,12 +57,12 @@ Vulnerability #2: GO-2021-0054 Fixed in: github.com/tidwall/gjson@v1.6.6 Example traces found: #1: for function github.com/tidwall/gjson.Result.ForEach - vuln.go:14:20: golang.org/vuln.main - gjson.go:297:12: github.com/tidwall/gjson.Result.Get - gjson.go:1881:36: github.com/tidwall/gjson.Get - gjson.go:2587:21: github.com/tidwall/gjson.execModifier - gjson.go:2631:21: github.com/tidwall/gjson.modPretty - gjson.go:220:17: github.com/tidwall/gjson.Result.ForEach + main @ golang.org/vuln/vuln.go:14:20 + Result.Get @ github.com/tidwall/gjson/gjson.go:297:12 + Get @ github.com/tidwall/gjson/gjson.go:1881:36 + execModifier @ github.com/tidwall/gjson/gjson.go:2587:21 + modPretty @ github.com/tidwall/gjson/gjson.go:2631:21 + Result.ForEach @ github.com/tidwall/gjson/gjson.go:220:17 Your code is affected by 2 vulnerabilities from 1 module. This scan also found 1 vulnerability in packages you import and 1 vulnerability diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_multientry_text.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_multientry_text.ct index 5e03a39f..2cde7834 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_multientry_text.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_multientry_text.ct @@ -43,14 +43,14 @@ Vulnerability #1: GO-2021-0113 Fixed in: golang.org/x/text@v0.3.7 Example traces found: #1: for function golang.org/x/text/language.MustParse - main.go:26:3: golang.org/multientry.main - main.go:48:8: golang.org/multientry.D - main.go:99:20: golang.org/multientry.foobar - language/tags.go:13:6: golang.org/x/text/language.MustParse + main @ golang.org/multientry/main.go:26:3 + D @ golang.org/multientry/main.go:48:8 + foobar @ golang.org/multientry/main.go:99:20 + MustParse @ golang.org/x/text/language/tags.go:13:6 #2: for function golang.org/x/text/language.Parse - main.go:22:3: golang.org/multientry.main - main.go:44:23: golang.org/multientry.C - language/parse.go:33:6: golang.org/x/text/language.Parse + main @ golang.org/multientry/main.go:22:3 + C @ golang.org/multientry/main.go:44:23 + Parse @ golang.org/x/text/language/parse.go:33:6 === Package Results === diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_subdir_text.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_subdir_text.ct index 299b38af..7b1986d8 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_subdir_text.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_subdir_text.ct @@ -43,8 +43,8 @@ Vulnerability #1: GO-2021-0265 Fixed in: github.com/tidwall/gjson@v1.9.3 Example traces found: #1: for function github.com/tidwall/gjson.Result.Get - subdir/subdir.go:8:20: golang.org/vuln/subdir.Foo - gjson.go:296:17: github.com/tidwall/gjson.Result.Get + Foo @ golang.org/vuln/subdir/subdir.go:8:20 + Result.Get @ github.com/tidwall/gjson/gjson.go:296:17 Vulnerability #2: GO-2021-0054 Due to improper bounds checking, maliciously crafted JSON objects can cause @@ -56,12 +56,12 @@ Vulnerability #2: GO-2021-0054 Fixed in: github.com/tidwall/gjson@v1.6.6 Example traces found: #1: for function github.com/tidwall/gjson.Result.ForEach - subdir/subdir.go:8:20: golang.org/vuln/subdir.Foo - gjson.go:297:12: github.com/tidwall/gjson.Result.Get - gjson.go:1881:36: github.com/tidwall/gjson.Get - gjson.go:2587:21: github.com/tidwall/gjson.execModifier - gjson.go:2631:21: github.com/tidwall/gjson.modPretty - gjson.go:220:17: github.com/tidwall/gjson.Result.ForEach + Foo @ golang.org/vuln/subdir/subdir.go:8:20 + Result.Get @ github.com/tidwall/gjson/gjson.go:297:12 + Get @ github.com/tidwall/gjson/gjson.go:1881:36 + execModifier @ github.com/tidwall/gjson/gjson.go:2587:21 + modPretty @ github.com/tidwall/gjson/gjson.go:2631:21 + Result.ForEach @ github.com/tidwall/gjson/gjson.go:220:17 Your code is affected by 2 vulnerabilities from 1 module. This scan found no other vulnerabilities in packages you import or modules you diff --git a/cmd/govulncheck/testdata/stdlib/config.json b/cmd/govulncheck/testdata/stdlib/config.json index eb8ae4e3..630175ab 100644 --- a/cmd/govulncheck/testdata/stdlib/config.json +++ b/cmd/govulncheck/testdata/stdlib/config.json @@ -1,8 +1,8 @@ { "fixups": [ { - "pattern": "\\.go:(\\d+):(\\d+):", - "replace": ".go:\u003cl\u003e:\u003cc\u003e:", + "pattern": "\\.go:(\\d+):(\\d+)", + "replace": ".go:\u003cl\u003e:\u003cc\u003e", "comment": " mask line and column with and placeholders, resp." }, { diff --git a/cmd/govulncheck/testdata/stdlib/testfiles/stdlib/source_stdlib_text.ct b/cmd/govulncheck/testdata/stdlib/testfiles/stdlib/source_stdlib_text.ct index 90ea9256..0463b2e4 100644 --- a/cmd/govulncheck/testdata/stdlib/testfiles/stdlib/source_stdlib_text.ct +++ b/cmd/govulncheck/testdata/stdlib/testfiles/stdlib/source_stdlib_text.ct @@ -35,12 +35,12 @@ Vulnerability #1: GO-2022-0969 Fixed in: net/http@go1.18.6 Example traces found: #1: for function net/http.ListenAndServe - stdlib.go::: golang.org/stdlib.main - src/net/http/server.go::: net/http.ListenAndServe + main @ golang.org/stdlib/stdlib.go:: + ListenAndServe @ stdlib/src/net/http/server.go:: #2: for function net/http.Serve - stdlib.go::: golang.org/stdlib.main - stdlib.go::: golang.org/stdlib.work[string] - src/net/http/server.go::: net/http.Serve + main @ golang.org/stdlib/stdlib.go:: + work[string] @ golang.org/stdlib/stdlib.go:: + Serve @ stdlib/src/net/http/server.go:: Your code is affected by 1 vulnerability from the Go standard library. This scan found no other vulnerabilities in packages you import or modules you diff --git a/cmd/govulncheck/testdata/strip/testfiles/binary/strip.ct b/cmd/govulncheck/testdata/strip/testfiles/binary/strip.ct index 0bdb3df8..2ade165e 100644 --- a/cmd/govulncheck/testdata/strip/testfiles/binary/strip.ct +++ b/cmd/govulncheck/testdata/strip/testfiles/binary/strip.ct @@ -51,29 +51,29 @@ Vulnerability #1: GO-2021-0113 Fixed in: golang.org/x/text@v0.3.7 Vulnerable symbols found: #1: for function golang.org/x/text/language.Compose - golang.org/x/text/language.Compose + Compose #2: for function golang.org/x/text/language.Make - golang.org/x/text/language.Make + Make #3: for function golang.org/x/text/language.MatchStrings - golang.org/x/text/language.MatchStrings + MatchStrings #4: for function golang.org/x/text/language.MustParse - golang.org/x/text/language.MustParse + MustParse #5: for function golang.org/x/text/language.Parse - golang.org/x/text/language.Parse + Parse #6: for function golang.org/x/text/language.ParseAcceptLanguage - golang.org/x/text/language.ParseAcceptLanguage + ParseAcceptLanguage #7: for function golang.org/x/text/language.Tag.Base - golang.org/x/text/language.Tag.Base + Tag.Base #8: for function golang.org/x/text/language.Tag.Extension - golang.org/x/text/language.Tag.Extension + Tag.Extension #9: for function golang.org/x/text/language.Tag.IsRoot - golang.org/x/text/language.Tag.IsRoot + Tag.IsRoot #10: for function golang.org/x/text/language.Tag.Parent - golang.org/x/text/language.Tag.Parent + Tag.Parent #11: for function golang.org/x/text/language.Tag.Region - golang.org/x/text/language.Tag.Region + Tag.Region #12: for function golang.org/x/text/language.Tag.String - golang.org/x/text/language.Tag.String + Tag.String Vulnerability #2: GO-2020-0015 Infinite loop when decoding some inputs in golang.org/x/text @@ -83,11 +83,11 @@ Vulnerability #2: GO-2020-0015 Fixed in: golang.org/x/text@v0.3.3 Vulnerable symbols found: #1: for function golang.org/x/text/transform.String - golang.org/x/text/transform.String + String #2: for function golang.org/x/text/encoding/unicode.bomOverride.Transform - golang.org/x/text/encoding/unicode.bomOverride.Transform + bomOverride.Transform #3: for function golang.org/x/text/encoding/unicode.utf16Decoder.Transform - golang.org/x/text/encoding/unicode.utf16Decoder.Transform + utf16Decoder.Transform Your code is affected by 2 vulnerabilities from 1 module. This scan found no other vulnerabilities in packages you import or modules you diff --git a/internal/scan/result_test.go b/internal/scan/result_test.go index 54207df0..82f3386b 100644 --- a/internal/scan/result_test.go +++ b/internal/scan/result_test.go @@ -62,7 +62,7 @@ func TestFrame(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { buf := &strings.Builder{} - addSymbolName(buf, test.frame, test.short) + addSymbol(buf, test.frame, test.short) got := buf.String() if got != test.wantFunc { t.Errorf("want %v func name; got %v", test.wantFunc, got) diff --git a/internal/scan/template.go b/internal/scan/template.go index 083c2b96..5c1a4a75 100644 --- a/internal/scan/template.go +++ b/internal/scan/template.go @@ -182,7 +182,13 @@ func posToString(p *govulncheck.Position) string { func symbol(frame *govulncheck.Frame, short bool) string { buf := &strings.Builder{} - addSymbolName(buf, frame, short) + addSymbol(buf, frame, short) + return buf.String() +} + +func symbolName(frame *govulncheck.Frame) string { + buf := &strings.Builder{} + addSymbolName(buf, frame) return buf.String() } @@ -210,12 +216,12 @@ func compactTrace(finding *govulncheck.Finding) string { if l > 1 { // print the root of the compact trace - addSymbolName(buf, compact[iTop], true) + addSymbol(buf, compact[iTop], true) buf.WriteString(" calls ") } if l > 2 { // print next element of the trace, if any - addSymbolName(buf, compact[iTop-1], true) + addSymbol(buf, compact[iTop-1], true) buf.WriteString(", which") if l > 3 { // don't print the third element, just acknowledge it @@ -223,7 +229,7 @@ func compactTrace(finding *govulncheck.Finding) string { } buf.WriteString(" calls ") } - addSymbolName(buf, compact[0], true) // print the vulnerable symbol + addSymbol(buf, compact[0], true) // print the vulnerable symbol return buf.String() } @@ -255,7 +261,7 @@ func importPathToAssumedName(importPath string) string { return base } -func addSymbolName(w io.Writer, frame *govulncheck.Frame, short bool) { +func addSymbol(w io.Writer, frame *govulncheck.Frame, short bool) { if frame.Function == "" { return } @@ -267,6 +273,10 @@ func addSymbolName(w io.Writer, frame *govulncheck.Frame, short bool) { io.WriteString(w, pkg) io.WriteString(w, ".") } + addSymbolName(w, frame) +} + +func addSymbolName(w io.Writer, frame *govulncheck.Frame) { if frame.Receiver != "" { if frame.Receiver[0] == '*' { io.WriteString(w, frame.Receiver[1:]) diff --git a/internal/scan/testdata/source_traces.txt b/internal/scan/testdata/source_traces.txt index e7b73c31..1eb7991b 100644 --- a/internal/scan/testdata/source_traces.txt +++ b/internal/scan/testdata/source_traces.txt @@ -9,8 +9,8 @@ Vulnerability #1: GO-0000-0001 Platforms: amd Example traces found: #1: for function vmod.Vuln - main.main - vmod.Vuln + main + Vuln Your code is affected by 1 vulnerability from the Go standard library. This scan also found 1 vulnerability in packages you import and 0 diff --git a/internal/scan/text.go b/internal/scan/text.go index 09804f78..6a51d2e3 100644 --- a/internal/scan/text.go +++ b/internal/scan/text.go @@ -335,15 +335,23 @@ func (h *TextHandler) traces(traces []*findingSummary) { for i := len(entry.Trace) - 1; i >= 0; i-- { t := entry.Trace[i] h.print(" ") + h.print(symbolName(t)) if t.Position != nil { - h.print(posToString(t.Position), ": ") + h.print(" @ ", symbolPath(t)) } - h.print(symbol(t, false), "\n") + h.print("\n") } } } } +// symbolPath returns a user-friendly path to a symbol. +func symbolPath(t *govulncheck.Frame) string { + // Add module path prefix to symbol paths to be more + // explicit to which module the symbols belong to. + return t.Module + "/" + posToString(t.Position) +} + func (h *TextHandler) summary(c summaryCounters) { // print short summary of findings identified at the desired level of scan precision var vulnCount int