[vuln] cmd/govulncheck: add verbose mode
49 views
Skip to first unread message
Jonathan Amsterdam (Gerrit)
May 23, 2022, 2:07:33 PM5/23/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Julie Qiu, Damien Neil, Gopher Robot, golang-co...@googlegroups.com
Jonathan Amsterdam submitted this change.
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: cmd/govulncheck/main.go
Insertions: 13, Deletions: 131.
@@ -27,11 +27,10 @@
"sort"
"strings"
- "golang.org/x/mod/semver"
"golang.org/x/tools/go/buildutil"
"golang.org/x/tools/go/packages"
"golang.org/x/vuln/client"
- "golang.org/x/vuln/osv"
+ "golang.org/x/vuln/cmd/govulncheck/internal/govulncheck"
"golang.org/x/vuln/vulncheck"
)
@@ -85,7 +84,9 @@
if GOVULNDB := os.Getenv("GOVULNDB"); GOVULNDB != "" {
dbs = strings.Split(GOVULNDB, ",")
}
- dbClient, err := client.NewClient(dbs, client.Options{HTTPCache: defaultCache()})
+ dbClient, err := client.NewClient(dbs, client.Options{
+ HTTPCache: govulncheck.DefaultCache(),
+ })
if err != nil {
die("govulncheck: %s", err)
}
@@ -94,9 +95,8 @@
patterns := flag.Args()
var (
- r *vulncheck.Result
- pkgs []*packages.Package
- vulns []*vulncheck.Vuln
+ r *vulncheck.Result
+ pkgs []*vulncheck.Package
)
if len(patterns) == 1 && isFile(patterns[0]) {
f, err := os.Open(patterns[0])
@@ -108,69 +108,41 @@
if err != nil {
die("govulncheck: %v", err)
}
- vulns = r.Vulns
} else {
cfg := &packages.Config{
- Mode: packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedImports | packages.NeedTypes | packages.NeedTypesSizes | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedDeps | packages.NeedModule,
Tests: *testsFlag,
BuildFlags: []string{fmt.Sprintf("-tags=%s", strings.Join(build.Default.BuildTags, ","))},
}
- pkgs, err = loadPackages(cfg, patterns)
+ pkgs, err = govulncheck.LoadPackages(cfg, patterns...)
if err != nil {
die("govulncheck: %v", err)
}
- r, err = vulncheck.Source(ctx, vulncheck.Convert(pkgs), vcfg)
+ r, err = govulncheck.Source(ctx, pkgs, dbClient)
if err != nil {
die("govulncheck: %v", err)
}
-
- // Skip vulns that are in the import graph but have no calls to them.
- for _, v := range r.Vulns {
- if v.CallSink != 0 {
- vulns = append(vulns, v)
- }
- }
}
if *jsonFlag {
writeJSON(r)
} else {
- callStacks := vulncheck.CallStacks(r)
- // Create set of top-level packages, used to find representative symbols
- topPackages := map[string]bool{}
- for _, p := range pkgs {
- topPackages[p.PkgPath] = true
- }
- vulnGroups := groupByIDAndPackage(vulns)
- moduleVersions := moduleVersionMap(r.Modules)
+ // set of top-level packages, used to find representative symbols
+ ci := govulncheck.GetCallInfo(r, pkgs)
if *htmlFlag {
- if err := html(os.Stdout, r, callStacks, moduleVersions, topPackages, vulnGroups); err != nil {
+ if err := html(os.Stdout, r, ci); err != nil {
die("writing HTML: %v", err)
}
} else {
- writeText(r, callStacks, moduleVersions, topPackages, vulnGroups)
+ writeText(r, ci)
}
}
exitCode := 0
// Following go vet, fail with 3 if there are findings (in this case, vulns).
- if len(vulns) > 0 {
+ if len(r.Vulns) > 0 {
exitCode = 3
}
os.Exit(exitCode)
}
-// moduleVersionMap builds a map from module paths to versions.
-func moduleVersionMap(mods []*vulncheck.Module) map[string]string {
- moduleVersions := map[string]string{}
- for _, m := range mods {
- v := m.Version
- if m.Replace != nil {
- v = m.Replace.Version
- }
- moduleVersions[m.Path] = v
- }
- return moduleVersions
-}
-
func writeJSON(r *vulncheck.Result) {
b, err := json.MarshalIndent(r, "", "\t")
if err != nil {
@@ -186,19 +158,18 @@
fmt.Printf("%-*s%s\n", labelWidth, label, text)
}
-func writeText(r *vulncheck.Result, callStacks map[*vulncheck.Vuln][]vulncheck.CallStack, moduleVersions map[string]string, topPackages map[string]bool, vulnGroups [][]*vulncheck.Vuln) {
-
- for _, vg := range vulnGroups {
+func writeText(r *vulncheck.Result, ci *govulncheck.CallInfo) {
+ for _, vg := range ci.VulnGroups {
// All the vulns in vg have the same PkgPath, ModPath and OSV.
// All have a non-zero CallSink.
v0 := vg[0]
writeLine("package:", v0.PkgPath)
- writeLine("your version:", moduleVersions[v0.ModPath])
- writeLine("fixed version:", "v"+latestFixed(v0.OSV.Affected))
+ writeLine("your version:", ci.ModuleVersions[v0.ModPath])
+ writeLine("fixed version:", "v"+govulncheck.LatestFixed(v0.OSV.Affected))
if *verboseFlag {
- writeCallStacksVerbose(vg, callStacks, topPackages)
+ writeCallStacksVerbose(vg, ci)
} else {
- writeCallStacksDefault(vg, callStacks, topPackages)
+ writeCallStacksDefault(vg, ci)
}
writeLine("reference:", fmt.Sprintf("https://pkg.go.dev/vuln/%s", v0.OSV.ID))
desc := strings.Split(wrap(v0.OSV.Details, 80-labelWidth), "\n")
@@ -213,12 +184,12 @@
}
}
-func writeCallStacksDefault(vg []*vulncheck.Vuln, callStacks map[*vulncheck.Vuln][]vulncheck.CallStack, topPackages map[string]bool) {
+func writeCallStacksDefault(vg []*vulncheck.Vuln, ci *govulncheck.CallInfo) {
var summaries []string
for _, v := range vg {
- if css := callStacks[v]; len(css) > 0 {
- if sum := summarizeCallStack(css[0], topPackages, v.PkgPath); sum != "" {
+ if css := ci.CallStacks[v]; len(css) > 0 {
+ if sum := govulncheck.SummarizeCallStack(css[0], ci.TopPackages, v.PkgPath); sum != "" {
summaries = append(summaries, sum)
}
}
@@ -233,13 +204,13 @@
}
}
-func writeCallStacksVerbose(vg []*vulncheck.Vuln, callStacks map[*vulncheck.Vuln][]vulncheck.CallStack, topPackages map[string]bool) {
+func writeCallStacksVerbose(vg []*vulncheck.Vuln, ci *govulncheck.CallInfo) {
// Display one full call stack for each vuln.
fmt.Println("call stacks:")
nMore := 0
i := 1
for _, v := range vg {
- css := callStacks[v]
+ css := ci.CallStacks[v]
if len(css) == 0 {
continue
}
@@ -257,30 +228,13 @@
func writeCallStack(cs vulncheck.CallStack) {
for _, e := range cs {
- fmt.Printf(" %s\n", funcName(e.Function))
+ fmt.Printf(" %s\n", govulncheck.FuncName(e.Function))
if e.Call != nil && e.Call.Pos != nil {
fmt.Printf(" %s\n", e.Call.Pos.String())
}
}
}
-func groupByIDAndPackage(vs []*vulncheck.Vuln) [][]*vulncheck.Vuln {
- groups := map[[2]string][]*vulncheck.Vuln{}
- for _, v := range vs {
- key := [2]string{v.OSV.ID, v.PkgPath}
- groups[key] = append(groups[key], v)
- }
-
- var res [][]*vulncheck.Vuln
- for _, g := range groups {
- res = append(res, g)
- }
- sort.Slice(res, func(i, j int) bool {
- return res[i][0].PkgPath < res[j][0].PkgPath
- })
- return res
-}
-
func packageModule(p *packages.Package) *packages.Module {
m := p.Module
if m == nil {
@@ -300,106 +254,6 @@
return !s.IsDir()
}
-func loadPackages(cfg *packages.Config, patterns []string) ([]*packages.Package, error) {
- pkgs, err := packages.Load(cfg, patterns...)
- if err != nil {
- return nil, err
- }
- if packages.PrintErrors(pkgs) > 0 {
- return nil, fmt.Errorf("packages contain errors")
- }
- return pkgs, nil
-}
-
-// latestFixed returns the latest fixed version in the list of affected ranges,
-// or the empty string if there are no fixed versions.
-func latestFixed(as []osv.Affected) string {
- v := ""
- for _, a := range as {
- for _, r := range a.Ranges {
- if r.Type == osv.TypeSemver {
- for _, e := range r.Events {
- if e.Fixed != "" && (v == "" || semver.Compare(e.Fixed, v) > 0) {
- v = e.Fixed
- }
- }
- }
- }
- }
- return v
-}
-
-// summarizeCallStack returns a short description of the call stack.
-// It uses one of two forms, depending on what the lowest function F in topPkgs
-// calls:
-// - If it calls a function V from the vulnerable package, then summarizeCallStack
-// returns "F calls V".
-// - If it calls a function G in some other package, which eventually calls V,
-// it returns "F calls G, which eventually calls V".
-//
-// If it can't find any of these functions, summarizeCallStack returns the empty string.
-func summarizeCallStack(cs vulncheck.CallStack, topPkgs map[string]bool, vulnPkg string) string {
- // Find the lowest function in the top packages.
- iTop := lowest(cs, func(e vulncheck.StackEntry) bool {
- return topPkgs[pkgPath(e.Function)]
- })
- if iTop < 0 {
- return ""
- }
- // Find the highest function in the vulnerable package that is below iTop.
- iVuln := highest(cs[iTop+1:], func(e vulncheck.StackEntry) bool {
- return pkgPath(e.Function) == vulnPkg
- })
- if iVuln < 0 {
- return ""
- }
- iVuln += iTop + 1 // adjust for slice in call to highest.
- topName := funcName(cs[iTop].Function)
- vulnName := funcName(cs[iVuln].Function)
- if iVuln == iTop+1 {
- return fmt.Sprintf("%s calls %s", topName, vulnName)
- }
- return fmt.Sprintf("%s calls %s, which eventually calls %s",
- topName, funcName(cs[iTop+1].Function), vulnName)
-}
-
-// highest returns the highest (one with the smallest index) entry in the call
-// stack for which f returns true.
-func highest(cs vulncheck.CallStack, f func(e vulncheck.StackEntry) bool) int {
- for i := 0; i < len(cs); i++ {
- if f(cs[i]) {
- return i
- }
- }
- return -1
-}
-
-// lowest returns the lowest (one with the largets index) entry in the call
-// stack for which f returns true.
-func lowest(cs vulncheck.CallStack, f func(e vulncheck.StackEntry) bool) int {
- for i := len(cs) - 1; i >= 0; i-- {
- if f(cs[i]) {
- return i
- }
- }
- return -1
-}
-
-func pkgPath(fn *vulncheck.FuncNode) string {
- if fn.PkgPath != "" {
- return fn.PkgPath
- }
- s := strings.TrimPrefix(fn.RecvType, "*")
- if i := strings.LastIndexByte(s, '.'); i > 0 {
- s = s[:i]
- }
- return s
-}
-
-func funcName(fn *vulncheck.FuncNode) string {
- return strings.TrimPrefix(fn.String(), "*")
-}
-
// compact replaces consecutive runs of equal elements with a single copy.
// This is like the uniq command found on Unix.
// compact modifies the contents of the slice s; it does not create a new slice.
```
Approvals:
Damien Neil: Looks good to me, approved
Julie Qiu: Looks good to me, approved
Jonathan Amsterdam: Run TryBots
cmd/govulncheck: add verbose mode
First attempt at verbose output for govulncheck, selected by
the -v flag.
The output is the same as default mode, except that instead of
summarized call stacks, we show full call stacks. Limit to
one per vulnerable symbol.
This required increasing the scope of the file path filter
in TestCommand, to include all output.
Change-Id: Ia5fc8db4906fc472a6ccf4ac87d440815f21ee26
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/406577
Reviewed-by: Damien Neil <dn...@google.com>
Run-TryBot: Jonathan Amsterdam <j...@google.com>
Reviewed-by: Julie Qiu <ju...@golang.org>
---
M cmd/govulncheck/main.go
M cmd/govulncheck/main_test.go
A cmd/govulncheck/testdata/verbose.ct
3 files changed, 133 insertions(+), 55 deletions(-)
diff --git a/cmd/govulncheck/main.go b/cmd/govulncheck/main.go
index 7e8a0f0..545c104 100644
--- a/cmd/govulncheck/main.go
+++ b/cmd/govulncheck/main.go
@@ -23,7 +23,6 @@
"flag"
"fmt"
"go/build"
- "log"
"os"
"sort"
"strings"
@@ -114,7 +113,7 @@
Tests: *testsFlag,
BuildFlags: []string{fmt.Sprintf("-tags=%s", strings.Join(build.Default.BuildTags, ","))},
}
- pkgs, err = loadPackages(cfg, patterns)
+ pkgs, err = govulncheck.LoadPackages(cfg, patterns...)
if err != nil {
die("govulncheck: %v", err)
}
@@ -153,48 +152,100 @@
fmt.Println()
}
-func writeText(r *vulncheck.Result, ci *govulncheck.CallInfo) {
+const labelWidth = 16
- const labelWidth = 16
- line := func(label, text string) {
- fmt.Printf("%-*s%s\n", labelWidth, label, text)
- }
+func writeLine(label, text string) {
+ fmt.Printf("%-*s%s\n", labelWidth, label, text)
+}
+
+func writeText(r *vulncheck.Result, ci *govulncheck.CallInfo) {
for _, vg := range ci.VulnGroups {
// All the vulns in vg have the same PkgPath, ModPath and OSV.
// All have a non-zero CallSink.
v0 := vg[0]
- line("package:", v0.PkgPath)
- line("your version:", ci.ModuleVersions[v0.ModPath])
- line("fixed version:", "v"+govulncheck.LatestFixed(v0.OSV.Affected))
- var summaries []string
- for _, v := range vg {
- if css := ci.CallStacks[v]; len(css) > 0 {
- if sum := govulncheck.SummarizeCallStack(css[0], ci.TopPackages, v.PkgPath); sum != "" {
- summaries = append(summaries, sum)
- }
- }
+ writeLine("package:", v0.PkgPath)
+ writeLine("your version:", ci.ModuleVersions[v0.ModPath])
+ writeLine("fixed version:", "v"+govulncheck.LatestFixed(v0.OSV.Affected))
+ if *verboseFlag {
+ writeCallStacksVerbose(vg, ci)
+ } else {
+ writeCallStacksDefault(vg, ci)
}
- if len(summaries) > 0 {
- sort.Strings(summaries)
- summaries = compact(summaries)
- line("sample call stacks:", "")
- for _, s := range summaries {
- line("", s)
- }
- }
- line("reference:", fmt.Sprintf("https://pkg.go.dev/vuln/%s", v0.OSV.ID))
+ writeLine("reference:", fmt.Sprintf("https://pkg.go.dev/vuln/%s", v0.OSV.ID))
desc := strings.Split(wrap(v0.OSV.Details, 80-labelWidth), "\n")
for i, l := range desc {
if i == 0 {
- line("description:", l)
+ writeLine("description:", l)
} else {
- line("", l)
+ writeLine("", l)
}
}
fmt.Println()
}
}
+func writeCallStacksDefault(vg []*vulncheck.Vuln, ci *govulncheck.CallInfo) {
+
+ var summaries []string
+ for _, v := range vg {
+ if css := ci.CallStacks[v]; len(css) > 0 {
+ if sum := govulncheck.SummarizeCallStack(css[0], ci.TopPackages, v.PkgPath); sum != "" {
+ summaries = append(summaries, sum)
+ }
+ }
+ }
+ if len(summaries) > 0 {
+ sort.Strings(summaries)
+ summaries = compact(summaries)
+ fmt.Println("sample call stacks:")
+ for _, s := range summaries {
+ writeLine("", s)
+ }
+ }
+}
+
+func writeCallStacksVerbose(vg []*vulncheck.Vuln, ci *govulncheck.CallInfo) {
+ // Display one full call stack for each vuln.
+ fmt.Println("call stacks:")
+ nMore := 0
+ i := 1
+ for _, v := range vg {
+ css := ci.CallStacks[v]
+ if len(css) == 0 {
+ continue
+ }
+ fmt.Printf(" #%d: for function %s\n", i, v.Symbol)
+ writeCallStack(css[0])
+ fmt.Println()
+ i++
+ nMore += len(css) - 1
+ }
+ if nMore > 0 {
+ fmt.Printf(" There are %d more call stacks available.\n", nMore)
+ fmt.Printf("To see all of them, pass the -json or -html flags.\n")
+ }
+}
+
+func writeCallStack(cs vulncheck.CallStack) {
+ for _, e := range cs {
+ fmt.Printf(" %s\n", govulncheck.FuncName(e.Function))
+ if e.Call != nil && e.Call.Pos != nil {
+ fmt.Printf(" %s\n", e.Call.Pos.String())
+ }
+ }
+}
+
+func packageModule(p *packages.Package) *packages.Module {
+ m := p.Module
+ if m == nil {
+ return nil
+ }
+ if r := m.Replace; r != nil {
+ return r
+ }
+ return m
+}
+
func isFile(path string) bool {
s, err := os.Stat(path)
if err != nil {
@@ -203,20 +254,6 @@
return !s.IsDir()
}
-func loadPackages(cfg *packages.Config, patterns []string) ([]*vulncheck.Package, error) {
- if *verboseFlag {
- log.Println("loading packages...")
- }
- pkgs, err := govulncheck.LoadPackages(cfg, patterns...)
- if err != nil {
- return nil, err
- }
- if *verboseFlag {
- log.Printf("\t%d loaded packages\n", len(pkgs))
- }
- return pkgs, nil
-}
-
// compact replaces consecutive runs of equal elements with a single copy.
// This is like the uniq command found on Unix.
// compact modifies the contents of the slice s; it does not create a new slice.
diff --git a/cmd/govulncheck/main_test.go b/cmd/govulncheck/main_test.go
index 28f3553..32dd1c2 100644
--- a/cmd/govulncheck/main_test.go
+++ b/cmd/govulncheck/main_test.go
@@ -56,12 +56,7 @@
}
cmd.Env = append(os.Environ(), "GOVULNDB=file://"+testDir+"/testdata/vulndb")
out, err := cmd.CombinedOutput()
- for _, arg := range args {
- if arg == "-json" {
- out = filterJSON(out)
- break
- }
- }
+ out = filterGoFilePaths(out)
return out, err
}
@@ -81,15 +76,16 @@
ts.Run(t, *update)
}
-var goFileRegexp = regexp.MustCompile(`"[^"]*\.go"`)
+var goFileRegexp = regexp.MustCompile(`[^\s"]*\.go[\s":]`)
-// filterJSON modifies paths to Go files by replacing their directory with "...".
-// For example, "/a/b/c.go" becomes ".../c.go".
-// This makes it possible to compare govulncheck JSON output across systems, because
-// Go filenames in JSON output include setup-specific paths.
-func filterJSON(data []byte) []byte {
+// filterGoFilePaths modifies paths to Go files by replacing their directory with "...".
+// For example,/a/b/c.go becomes .../c.go .
+// This makes it possible to compare govulncheck output across systems, because
+// Go filenames include setup-specific paths.
+func filterGoFilePaths(data []byte) []byte {
return goFileRegexp.ReplaceAllFunc(data, func(b []byte) []byte {
- return []byte(fmt.Sprintf(`".../%s"`, filepath.Base(string(b)[1:len(b)-1])))
+ s := string(b)
+ return []byte(fmt.Sprintf(`.../%s%c`, filepath.Base(s[1:len(s)-1]), s[len(s)-1]))
})
}
diff --git a/cmd/govulncheck/testdata/verbose.ct b/cmd/govulncheck/testdata/verbose.ct
new file mode 100644
index 0000000..95c1311
--- /dev/null
+++ b/cmd/govulncheck/testdata/verbose.ct
@@ -0,0 +1,22 @@
+# Test of verbose mode.
+
+# No vulnerabilities, no output.
+$ cdmodule novuln
+$ govulncheck -v .
+
+$ cdmodule vuln
+$ govulncheck -v . --> FAIL 3
+package: golang.org/x/text/language
+your version: v0.3.0
+fixed version: v0.3.7
+call stacks:
+ #1: for function Parse
+ vuln.main
+ .../vuln.go:11:16
+ golang.org/x/text/language.Parse
+
+reference: https://pkg.go.dev/vuln/GO-2021-0113
+description: Due to improper index calculation, an incorrectly formatted
+ language tag can cause Parse to panic via an out of bounds read.
+ If Parse is used to process untrusted user inputs, this may be
+ used as a vector for a denial of service attack.
To view, visit change 406577. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages