diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 65fa5c5ff7..f868a48936 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -296,6 +296,7 @@ Result: "CurrentVersion": string, "FixedVersion": string, "CallStacks": [][]golang.org/x/tools/internal/lsp/command.StackEntry, + "CallStackSummaries": []string, }, } ``` diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index 06ddacd500..3fd9d03682 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -9,7 +9,6 @@ package vulncheck import ( "context" - "fmt" "log" "os" "strings" @@ -68,25 +67,13 @@ type cmd struct { // Run runs the govulncheck after loading packages using the provided packages.Config. func (c *cmd) Run(ctx context.Context, cfg *packages.Config, patterns ...string) (_ []Vuln, err error) { - // TODO: how&where can we ensure cfg is the right config for the given patterns? - - // vulncheck.Source may panic if the packages are incomplete. (e.g. broken code or failed dependency fetch) - defer func() { - if r := recover(); r != nil { - err = fmt.Errorf("cannot run vulncheck: %v", r) - } - }() - return c.run(ctx, cfg, patterns) -} - -func (c *cmd) run(ctx context.Context, packagesCfg *packages.Config, patterns []string) ([]Vuln, error) { - packagesCfg.Mode |= packages.NeedModule | packages.NeedName | packages.NeedFiles | + cfg.Mode |= packages.NeedModule | packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedImports | packages.NeedTypes | packages.NeedTypesSizes | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedDeps log.Println("loading packages...") - loadedPkgs, err := packages.Load(packagesCfg, patterns...) + loadedPkgs, err := packages.Load(cfg, patterns...) if err != nil { log.Printf("package load failed: %v", err) return nil, err @@ -94,53 +81,58 @@ func (c *cmd) run(ctx context.Context, packagesCfg *packages.Config, patterns [] log.Printf("loaded %d packages\n", len(loadedPkgs)) pkgs := vulncheck.Convert(loadedPkgs) - res, err := vulncheck.Source(ctx, pkgs, &vulncheck.Config{ - Client: c.Client, - ImportsOnly: false, + r, err := vulncheck.Source(ctx, pkgs, &vulncheck.Config{ + Client: c.Client, }) - cs := vulncheck.CallStacks(res) + if err != nil { + return nil, err + } - return toVulns(loadedPkgs, cs) + // Skip vulns that are in the import graph but have no calls to them. + var vulns []*vulncheck.Vuln + for _, v := range r.Vulns { + if v.CallSink != 0 { + vulns = append(vulns, v) + } + } + 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) + + return toVulns(callStacks, moduleVersions, topPackages, vulnGroups) // TODO: add import graphs. } -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 toVulns(pkgs []*packages.Package, callstacks map[*vulncheck.Vuln][]vulncheck.CallStack) ([]Vuln, error) { - // Build a map from module paths to versions. - moduleVersions := map[string]string{} - packages.Visit(pkgs, nil, func(p *packages.Package) { - if m := packageModule(p); m != nil { - moduleVersions[m.Path] = m.Version - } - }) - +func toVulns(callStacks map[*vulncheck.Vuln][]vulncheck.CallStack, moduleVersions map[string]string, topPackages map[string]bool, vulnGroups [][]*vulncheck.Vuln) ([]Vuln, error) { var vulns []Vuln - for v, trace := range callstacks { - if len(trace) == 0 { - continue - } + + for _, vg := range vulnGroups { + v0 := vg[0] vuln := Vuln{ - ID: v.OSV.ID, - Details: v.OSV.Details, - Aliases: v.OSV.Aliases, - Symbol: v.Symbol, - PkgPath: v.PkgPath, - ModPath: v.ModPath, - URL: href(v.OSV), - CurrentVersion: moduleVersions[v.ModPath], - FixedVersion: fixedVersion(v.OSV), - CallStacks: toCallStacks(trace), + ID: v0.OSV.ID, + PkgPath: v0.PkgPath, + CurrentVersion: moduleVersions[v0.ModPath], + FixedVersion: latestFixed(v0.OSV.Affected), + Details: v0.OSV.Details, + + Aliases: v0.OSV.Aliases, + Symbol: v0.Symbol, + ModPath: v0.ModPath, + URL: href(v0.OSV), + } + + // Keep first call stack for each vuln. + for _, v := range vg { + if css := callStacks[v]; len(css) > 0 { + vuln.CallStacks = append(vuln.CallStacks, toCallStack(css[0])) + vuln.CallStackSummaries = append(vuln.CallStackSummaries, summarizeCallStack(css[0], topPackages, v.PkgPath)) + } } vulns = append(vulns, vuln) } diff --git a/gopls/internal/vulncheck/command_test.go b/gopls/internal/vulncheck/command_test.go index a72e2e0a9d..f689ab9672 100644 --- a/gopls/internal/vulncheck/command_test.go +++ b/gopls/internal/vulncheck/command_test.go @@ -25,7 +25,6 @@ import ( "golang.org/x/tools/internal/lsp/tests" "golang.org/x/vuln/client" "golang.org/x/vuln/osv" - "golang.org/x/vuln/vulncheck" ) func TestCmd_Run(t *testing.T) { @@ -54,42 +53,31 @@ func TestCmd_Run(t *testing.T) { URL: "https://pkg.go.dev/vuln/GO-2022-01", CurrentVersion: "v1.1.3", FixedVersion: "v1.0.4", + CallStackSummaries: []string{ + "golang.org/entry/x.X calls golang.org/amod/avuln.VulnData.Vuln1", + "golang.org/entry/x.X calls golang.org/cmod/c.C1, which eventually calls golang.org/amod/avuln.VulnData.Vuln2", + }, }, CallStacksStr: []string{ - "golang.org/cmod/c.I.t0 called from golang.org/entry/x.X [approx.] (x.go:8)\n" + + "golang.org/entry/x.X [approx.] (x.go:8)\n" + "golang.org/amod/avuln.VulnData.Vuln1 (avuln.go:3)\n", - }, - }, - { - Vuln: Vuln{ - ID: "GO-2022-01", - Symbol: "VulnData.Vuln2", - PkgPath: "golang.org/amod/avuln", - ModPath: "golang.org/amod", - URL: "https://pkg.go.dev/vuln/GO-2022-01", - CurrentVersion: "v1.1.3", - FixedVersion: "v1.0.4", - }, - CallStacksStr: []string{ - "C1 called from golang.org/entry/x.X (x.go:8)\n" + - "Vuln2 called from golang.org/cmod/c.C1 (c.go:13)\n" + + "golang.org/entry/x.X (x.go:8)\n" + + "golang.org/cmod/c.C1 (c.go:13)\n" + "golang.org/amod/avuln.VulnData.Vuln2 (avuln.go:4)\n", }, }, { Vuln: Vuln{ - ID: "GO-2022-02", - Symbol: "Vuln", - PkgPath: "golang.org/bmod/bvuln", - ModPath: "golang.org/bmod", - URL: "https://pkg.go.dev/vuln/GO-2022-02", - CurrentVersion: "v0.5.0", + ID: "GO-2022-02", + Symbol: "Vuln", + PkgPath: "golang.org/bmod/bvuln", + ModPath: "golang.org/bmod", + URL: "https://pkg.go.dev/vuln/GO-2022-02", + CurrentVersion: "v0.5.0", + CallStackSummaries: []string{"golang.org/entry/y.Y calls golang.org/bmod/bvuln.Vuln"}, }, CallStacksStr: []string{ - "t0 called from golang.org/entry/y.Y [approx.] (y.go:5)\n" + - "golang.org/bmod/bvuln.Vuln (bvuln.go:2)\n", - "Y called from golang.org/entry/x.CallY (x.go:12)\n" + - "t0 called from golang.org/entry/y.Y [approx.] (y.go:5)\n" + + "golang.org/entry/y.Y [approx.] (y.go:5)\n" + "golang.org/bmod/bvuln.Vuln (bvuln.go:2)\n", }, }, @@ -97,8 +85,8 @@ func TestCmd_Run(t *testing.T) { // sort reports for stability before comparison. for _, rpts := range [][]report{got, want} { sort.Slice(rpts, func(i, j int) bool { - a, b := got[i], got[j] - if b.ID != b.ID { + a, b := rpts[i], rpts[j] + if a.ID != b.ID { return a.ID < b.ID } if a.PkgPath != b.PkgPath { @@ -254,50 +242,6 @@ var testClient1 = &mockClient{ }, } -var goldenReport1 = []string{` -{ - ID: "GO-2022-01", - Symbol: "VulnData.Vuln1", - PkgPath: "golang.org/amod/avuln", - ModPath: "golang.org/amod", - URL: "https://pkg.go.dev/vuln/GO-2022-01", - CurrentVersion "v1.1.3", - FixedVersion "v1.0.4", - "call_stacks": [ - "golang.org/cmod/c.I.t0 called from golang.org/entry/x.X [approx.] (x.go:8)\ngolang.org/amod/avuln.VulnData.Vuln1 (avuln.go:3)\n\n" - ] -} -`, - ` -{ - "id": "GO-2022-02", - "symbol": "Vuln", - "pkg_path": "golang.org/bmod/bvuln", - "mod_path": "golang.org/bmod", - "url": "https://pkg.go.dev/vuln/GO-2022-02", - "current_version": "v0.5.0", - "call_stacks": [ - "t0 called from golang.org/entry/y.Y [approx.] (y.go:5)\ngolang.org/bmod/bvuln.Vuln (bvuln.go:2)\n\n", - "Y called from golang.org/entry/x.CallY (x.go:12)\nt0 called from golang.org/entry/y.Y [approx.] (y.go:5)\ngolang.org/bmod/bvuln.Vuln (bvuln.go:2)\n\n" - ] -} -`, - ` -{ - "id": "GO-2022-01", - "symbol": "VulnData.Vuln2", - "pkg_path": "golang.org/amod/avuln", - "mod_path": "golang.org/amod", - "url": "https://pkg.go.dev/vuln/GO-2022-01", - "current_version": "v1.1.3", - FixedVersion: "v1.0.4", - "call_stacks": [ - "C1 called from golang.org/entry/x.X (x.go:8)\nVuln2 called from golang.org/cmod/c.C1 (c.go:13)\ngolang.org/amod/avuln.VulnData.Vuln2 (avuln.go:4)\n\n" - ] -} -`, -} - type mockClient struct { client.Client ret map[string][]*osv.Entry @@ -347,19 +291,6 @@ func runTest(t *testing.T, workspaceData, proxyData string, test func(context.Co test(ctx, snapshot) } -func sortStrs(s []string) []string { - sort.Strings(s) - return s -} - -func pkgPaths(pkgs []*vulncheck.Package) []string { - var r []string - for _, p := range pkgs { - r = append(r, p.PkgPath) - } - return sortStrs(r) -} - // TODO: expose this as a method of Snapshot. func packagesCfg(ctx context.Context, snapshot source.Snapshot) *packages.Config { view := snapshot.View() diff --git a/gopls/internal/vulncheck/util.go b/gopls/internal/vulncheck/util.go index a85b55bb04..e2a437b8c6 100644 --- a/gopls/internal/vulncheck/util.go +++ b/gopls/internal/vulncheck/util.go @@ -10,47 +10,132 @@ package vulncheck import ( "fmt" "go/token" + "sort" "strings" + "golang.org/x/mod/semver" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/vuln/osv" "golang.org/x/vuln/vulncheck" ) -// fixedVersion returns the semantic version of the module -// version with a fix. The semantic version is -// as defined by SemVer 2.0.0, with no leading “v” prefix. -// Returns an empty string if there is no reported fix. -func fixedVersion(info *osv.Entry) string { - var fixed string - for _, a := range info.Affected { +// TODO(hyangah): automate copy of golang.org/x/vuln/cmd. + +// 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 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 +} + +// 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 != "SEMVER" { - continue - } - for _, e := range r.Events { - if e.Fixed != "" { - // assuming the later entry has higher semver. - // TODO: check assumption. - fixed = "v" + e.Fixed + if r.Type == osv.TypeSemver { + for _, e := range r.Events { + if e.Fixed != "" && (v == "" || semver.Compare(e.Fixed, v) > 0) { + v = e.Fixed + } } } } } - return fixed + if v == "" || v[0] == 'v' { + return v + } + return "v" + v } -const maxNumCallStacks = 64 +// 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) +} -func toCallStacks(src []vulncheck.CallStack) []CallStack { - if len(src) > maxNumCallStacks { - src = src[:maxNumCallStacks] +// 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 + } } - var dest []CallStack - for _, s := range src { - dest = append(dest, toCallStack(s)) + 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 dest + 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 toCallStack(src vulncheck.CallStack) CallStack { @@ -66,8 +151,7 @@ func toStackEntry(src vulncheck.StackEntry) StackEntry { pos := f.Pos desc := funcName(f) if src.Call != nil { - pos = src.Call.Pos - desc = funcNameInCallSite(call) + " called from " + desc + pos = src.Call.Pos // Exact call site position is helpful. if !call.Resolved { // In case of a statically unresolved call site, communicate to the client // that this was approximately resolved to f @@ -86,13 +170,6 @@ func funcName(fn *vulncheck.FuncNode) string { return strings.TrimPrefix(fn.String(), "*") } -func funcNameInCallSite(call *vulncheck.CallSite) string { - if call.RecvType == "" { - return call.Name - } - return fmt.Sprintf("%s.%s", call.RecvType, call.Name) -} - // href returns a URL embedded in the entry if any. // If no suitable URL is found, it returns a default entry in // pkg.go.dev/vuln. diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index 9aecfbe780..8e4b1056d3 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -380,5 +380,8 @@ type Vuln struct { // Example call stacks. CallStacks []CallStack `json:",omitempty"` + // Short description of each call stack in CallStacks. + CallStackSummaries []string `json:",omitempty"` + // TODO: import graph & module graph. } diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index c2c1f82426..0695efc2fa 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -675,7 +675,7 @@ var GeneratedAPIJSON = &APIJSON{ Title: "Run vulncheck (experimental)", Doc: "Run vulnerability check (`govulncheck`).", ArgDoc: "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}", - ResultDoc: "{\n\t\"Vuln\": []{\n\t\t\"ID\": string,\n\t\t\"Details\": string,\n\t\t\"Aliases\": []string,\n\t\t\"Symbol\": string,\n\t\t\"PkgPath\": string,\n\t\t\"ModPath\": string,\n\t\t\"URL\": string,\n\t\t\"CurrentVersion\": string,\n\t\t\"FixedVersion\": string,\n\t\t\"CallStacks\": [][]golang.org/x/tools/internal/lsp/command.StackEntry,\n\t},\n}", + ResultDoc: "{\n\t\"Vuln\": []{\n\t\t\"ID\": string,\n\t\t\"Details\": string,\n\t\t\"Aliases\": []string,\n\t\t\"Symbol\": string,\n\t\t\"PkgPath\": string,\n\t\t\"ModPath\": string,\n\t\t\"URL\": string,\n\t\t\"CurrentVersion\": string,\n\t\t\"FixedVersion\": string,\n\t\t\"CallStacks\": [][]golang.org/x/tools/internal/lsp/command.StackEntry,\n\t\t\"CallStackSummaries\": []string,\n\t},\n}", }, { Command: "gopls.start_debugging",