From d097bc9f9d0168a89c8d85f5e2110d54bcbab78e Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Wed, 15 Jun 2022 18:51:42 -0400 Subject: [PATCH] gopls/internal/vulncheck: include nonaffecting vulnerability info This info is still useful to tell users that some required modules have known vulnerabilities, but the analyzed packages/workspaces are not affected. Those vulnerabilities are missing Symbol/PkgPath/CallStacks. Change-Id: I94ea0d8f9ebcb1270e05f055caff2a18ebacd034 Reviewed-on: https://go-review.googlesource.com/c/tools/+/412457 Reviewed-by: Jonathan Amsterdam --- gopls/internal/vulncheck/command.go | 88 +++++++++++++++++++++--- gopls/internal/vulncheck/command_test.go | 24 +++++++ internal/lsp/command/interface.go | 2 + 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index a89354f67e..53bf0f0386 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -11,12 +11,15 @@ import ( "context" "log" "os" + "sort" "strings" "golang.org/x/tools/go/packages" gvc "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/internal/lsp/command" "golang.org/x/vuln/client" + "golang.org/x/vuln/osv" + "golang.org/x/vuln/vulncheck" ) func init() { @@ -79,29 +82,84 @@ func (c *cmd) Run(ctx context.Context, cfg *packages.Config, patterns ...string) } log.Printf("loaded %d packages\n", len(loadedPkgs)) - r, err := gvc.Source(ctx, loadedPkgs, c.Client) + log.Printf("analyzing %d packages...\n", len(loadedPkgs)) + + r, err := vulncheck.Source(ctx, loadedPkgs, &vulncheck.Config{Client: c.Client}) if err != nil { return nil, err } + unaffectedMods := filterUnaffected(r.Vulns) + r.Vulns = filterCalled(r) + callInfo := gvc.GetCallInfo(r, loadedPkgs) - return toVulns(callInfo) + return toVulns(callInfo, unaffectedMods) // TODO: add import graphs. } -func toVulns(ci *gvc.CallInfo) ([]Vuln, error) { +// filterCalled returns vulnerabilities where the symbols are actually called. +func filterCalled(r *vulncheck.Result) []*vulncheck.Vuln { + var vulns []*vulncheck.Vuln + for _, v := range r.Vulns { + if v.CallSink != 0 { + vulns = append(vulns, v) + } + } + return vulns +} + +// filterUnaffected returns vulnerabilities where no symbols are called, +// grouped by module. +func filterUnaffected(vulns []*vulncheck.Vuln) map[string][]*osv.Entry { + // It is possible that the same vuln.OSV.ID has vuln.CallSink != 0 + // for one symbol, but vuln.CallSink == 0 for a different one, so + // we need to filter out ones that have been called. + called := map[string]bool{} + for _, vuln := range vulns { + if vuln.CallSink != 0 { + called[vuln.OSV.ID] = true + } + } + + modToIDs := map[string]map[string]*osv.Entry{} + for _, vuln := range vulns { + if !called[vuln.OSV.ID] { + if _, ok := modToIDs[vuln.ModPath]; !ok { + modToIDs[vuln.ModPath] = map[string]*osv.Entry{} + } + // keep only one vuln.OSV instance for the same ID. + modToIDs[vuln.ModPath][vuln.OSV.ID] = vuln.OSV + } + } + output := map[string][]*osv.Entry{} + for m, vulnSet := range modToIDs { + var vulns []*osv.Entry + for _, vuln := range vulnSet { + vulns = append(vulns, vuln) + } + sort.Slice(vulns, func(i, j int) bool { return vulns[i].ID < vulns[j].ID }) + output[m] = vulns + } + return output +} + +func fixed(v *osv.Entry) string { + lf := gvc.LatestFixed(v.Affected) + if lf != "" && lf[0] != 'v' { + lf = "v" + lf + } + return lf +} + +func toVulns(ci *gvc.CallInfo, unaffectedMods map[string][]*osv.Entry) ([]Vuln, error) { var vulns []Vuln for _, vg := range ci.VulnGroups { v0 := vg[0] - lf := gvc.LatestFixed(v0.OSV.Affected) - if lf != "" && lf[0] != 'v' { - lf = "v" + lf - } vuln := Vuln{ ID: v0.OSV.ID, PkgPath: v0.PkgPath, CurrentVersion: ci.ModuleVersions[v0.ModPath], - FixedVersion: lf, + FixedVersion: fixed(v0.OSV), Details: v0.OSV.Details, Aliases: v0.OSV.Aliases, @@ -119,5 +177,19 @@ func toVulns(ci *gvc.CallInfo) ([]Vuln, error) { } vulns = append(vulns, vuln) } + for m, vg := range unaffectedMods { + for _, v0 := range vg { + vuln := Vuln{ + ID: v0.ID, + Details: v0.Details, + Aliases: v0.Aliases, + ModPath: m, + URL: href(v0), + CurrentVersion: "", + FixedVersion: fixed(v0), + } + vulns = append(vulns, vuln) + } + } return vulns, nil } diff --git a/gopls/internal/vulncheck/command_test.go b/gopls/internal/vulncheck/command_test.go index f689ab9672..f6e2d1b761 100644 --- a/gopls/internal/vulncheck/command_test.go +++ b/gopls/internal/vulncheck/command_test.go @@ -81,6 +81,15 @@ func TestCmd_Run(t *testing.T) { "golang.org/bmod/bvuln.Vuln (bvuln.go:2)\n", }, }, + { + Vuln: Vuln{ + ID: "GO-2022-03", + Details: "unaffecting vulnerability", + ModPath: "golang.org/amod", + URL: "https://pkg.go.dev/vuln/GO-2022-03", + FixedVersion: "v1.0.4", + }, + }, } // sort reports for stability before comparison. for _, rpts := range [][]report{got, want} { @@ -228,6 +237,21 @@ var testClient1 = &mockClient{ EcosystemSpecific: osv.EcosystemSpecific{Symbols: []string{"VulnData.Vuln1", "VulnData.Vuln2"}}, }}, }, + { + ID: "GO-2022-03", + Details: "unaffecting vulnerability", + References: []osv.Reference{ + { + Type: "href", + URL: "pkg.go.dev/vuln/GO-2022-01", + }, + }, + Affected: []osv.Affected{{ + Package: osv.Package{Name: "golang.org/amod/avuln"}, + Ranges: osv.Affects{{Type: osv.TypeSemver, Events: []osv.RangeEvent{{Introduced: "1.0.0"}, {Fixed: "1.0.4"}, {Introduced: "1.1.2"}}}}, + EcosystemSpecific: osv.EcosystemSpecific{Symbols: []string{"nonExisting"}}, + }}, + }, }, "golang.org/bmod": { { diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go index 8e4b1056d3..1f3b092fab 100644 --- a/internal/lsp/command/interface.go +++ b/internal/lsp/command/interface.go @@ -359,8 +359,10 @@ type Vuln struct { Aliases []string `json:",omitempty"` // Symbol is the name of the detected vulnerable function or method. + // Can be empty if the vulnerability exists in required modules, but no vulnerable symbols are used. Symbol string `json:",omitempty"` // PkgPath is the package path of the detected Symbol. + // Can be empty if the vulnerability exists in required modules, but no vulnerable packages are used. PkgPath string `json:",omitempty"` // ModPath is the module path corresponding to PkgPath. // TODO: how do we specify standard library's vulnerability?