Skip to content

Commit

Permalink
gopls/internal/vulncheck: include nonaffecting vulnerability info
Browse files Browse the repository at this point in the history
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 <jba@google.com>
  • Loading branch information
hyangah committed Jun 16, 2022
1 parent e8b9ff1 commit d097bc9
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 8 deletions.
88 changes: 80 additions & 8 deletions gopls/internal/vulncheck/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
24 changes: 24 additions & 0 deletions gopls/internal/vulncheck/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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} {
Expand Down Expand Up @@ -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": {
{
Expand Down
2 changes: 2 additions & 0 deletions internal/lsp/command/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down

0 comments on commit d097bc9

Please sign in to comment.