exp/internal/audit: save audit results in a dedicated data structure
Encapsulates results of govulncheck search into a specialized and
self-contained data structure that groups findings by vulnerability and
sorts them by their estimated usefulness to the user.
Change-Id: I03555c97d892e53c7346680c8e54f7a00b0fd264
Reviewed-on: https://go-review.googlesource.com/c/exp/+/338290
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
diff --git a/vulndb/govulncheck/main.go b/vulndb/govulncheck/main.go
index 092ce4c..4ea2589 100644
--- a/vulndb/govulncheck/main.go
+++ b/vulndb/govulncheck/main.go
@@ -19,11 +19,9 @@
"encoding/json"
"flag"
"fmt"
- "io"
"log"
"os"
"runtime"
- "sort"
"strings"
"golang.org/x/exp/vulndb/internal/audit"
@@ -31,7 +29,6 @@
"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/ssa/ssautil"
"golang.org/x/vulndb/client"
- "golang.org/x/vulndb/osv"
)
var (
@@ -68,59 +65,6 @@
databases are merged.
`
-type results struct {
- Modules []*packages.Module
- Vulns []*osv.Entry
- Findings []audit.Finding
-}
-
-func (r *results) unreachable() []*osv.Entry {
- seen := map[string]bool{}
- for _, f := range r.Findings {
- for _, v := range f.Vulns {
- seen[v.ID] = true
- }
- }
- unseen := []*osv.Entry{}
- for _, v := range r.Vulns {
- if seen[v.ID] {
- continue
- }
- unseen = append(unseen, v)
- }
- return unseen
-}
-
-// presentTo pretty-prints results to out.
-func (r *results) presentTo(out io.Writer) {
- sort.Slice(r.Vulns, func(i, j int) bool { return r.Vulns[i].ID < r.Vulns[j].ID })
- sort.SliceStable(r.Findings, func(i int, j int) bool { return audit.FindingCompare(r.Findings[i], r.Findings[j]) })
- if !*jsonFlag {
- for _, finding := range r.Findings {
- finding.Write(out)
- out.Write([]byte{'\n'})
- }
- if unreachable := r.unreachable(); len(unreachable) > 0 {
- fmt.Fprintf(out, "The following %d vulnerabilities don't affect this project:\n", len(unreachable))
- for _, u := range unreachable {
- var aliases string
- if len(u.Aliases) > 0 {
- aliases = fmt.Sprintf(" (%s)", strings.Join(u.Aliases, ", "))
- }
- fmt.Fprintf(out, "- %s%s (package imported, but vulnerable symbol is not reachable)\n", u.ID, aliases)
- }
- }
- return
- }
- b, err := json.MarshalIndent(r, "", "\t")
- if err != nil {
- fmt.Fprintf(os.Stderr, "govulncheck: %s\n", err)
- os.Exit(1)
- }
- out.Write(b)
- out.Write([]byte{'\n'})
-}
-
func main() {
flag.Usage = func() { fmt.Fprintln(os.Stderr, usage) }
flag.Parse()
@@ -145,7 +89,22 @@
os.Exit(1)
}
- r.presentTo(os.Stdout)
+ writeOut(r, *jsonFlag)
+}
+
+func writeOut(r *audit.Results, toJson bool) {
+ if !toJson {
+ os.Stdout.Write([]byte(r.String()))
+ return
+ }
+
+ b, err := json.MarshalIndent(r, "", "\t")
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "govulncheck: %s\n", err)
+ os.Exit(1)
+ }
+ os.Stdout.Write(b)
+ os.Stdout.Write([]byte{'\n'})
}
// extractModules collects modules in `pkgs` up to uniqueness of
@@ -192,8 +151,7 @@
return !s.IsDir()
}
-func run(cfg *packages.Config, patterns []string, importsOnly bool, dbs []string) (*results, error) {
- r := &results{}
+func run(cfg *packages.Config, patterns []string, importsOnly bool, dbs []string) (*audit.Results, error) {
if len(patterns) == 1 && isFile(patterns[0]) {
modules, symbols, err := binscan.ExtractPackagesAndSymbols(patterns[0])
if err != nil {
@@ -204,14 +162,15 @@
if err != nil {
return nil, fmt.Errorf("failed to create database client: %s", err)
}
+
vulns, err := audit.FetchVulnerabilities(dbClient, modules)
if err != nil {
return nil, fmt.Errorf("failed to load vulnerability dbs: %v", err)
}
vulns = vulns.Filter(runtime.GOOS, runtime.GOARCH)
- r.Findings = audit.VulnerablePackageSymbols(symbols, vulns)
- return r, nil
+ results := audit.VulnerablePackageSymbols(symbols, vulns)
+ return &results, nil
}
// Load packages.
@@ -233,17 +192,16 @@
if *verboseFlag {
log.Println("loading database...")
}
- r.Modules = extractModules(pkgs)
dbClient, err := client.NewClient(dbs, client.Options{})
if err != nil {
return nil, fmt.Errorf("failed to create database client: %s", err)
}
- modVulns, err := audit.FetchVulnerabilities(dbClient, r.Modules)
+
+ modVulns, err := audit.FetchVulnerabilities(dbClient, extractModules(pkgs))
if err != nil {
return nil, fmt.Errorf("failed to fetch vulnerabilities: %v", err)
}
modVulns = modVulns.Filter(runtime.GOOS, runtime.GOARCH)
-
if *verboseFlag {
log.Printf("\t%d known vulnerabilities.\n", modVulns.Num())
}
@@ -262,15 +220,11 @@
if *verboseFlag {
log.Println("detecting vulnerabilities...")
}
- var findings []audit.Finding
+ var results audit.Results
if importsOnly {
- r.Findings = audit.VulnerableImports(ssaPkgs, modVulns)
+ results = audit.VulnerableImports(ssaPkgs, modVulns)
} else {
- r.Findings = audit.VulnerableSymbols(ssaPkgs, modVulns)
+ results = audit.VulnerableSymbols(ssaPkgs, modVulns)
}
- if *verboseFlag {
- log.Printf("\t%d detected findings.\n", len(findings))
- }
-
- return r, nil
+ return &results, nil
}
diff --git a/vulndb/govulncheck/main_test.go b/vulndb/govulncheck/main_test.go
index e762cc6..00c069a 100644
--- a/vulndb/govulncheck/main_test.go
+++ b/vulndb/govulncheck/main_test.go
@@ -15,16 +15,13 @@
"os/exec"
"path"
"path/filepath"
- "reflect"
"runtime"
- "sort"
"strings"
"testing"
"golang.org/x/exp/vulndb/internal/audit"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/go/packages/packagestest"
- "golang.org/x/vulndb/osv"
)
// TODO(zpavlinovic): improve integration tests.
@@ -144,6 +141,16 @@
return true
}
+func allFindings(r *audit.Results) []audit.Finding {
+ var findings []audit.Finding
+ for _, v := range r.Vulnerabilities {
+ for _, f := range r.VulnFindings[v.ID] {
+ findings = append(findings, f)
+ }
+ }
+ return findings
+}
+
func TestHashicorpVault(t *testing.T) {
if testing.Short() {
t.Skip("skipping test in short mode.")
@@ -220,8 +227,7 @@
if err != nil {
t.Fatal(err)
}
- sort.SliceStable(r.Findings, func(i int, j int) bool { return audit.FindingCompare(r.Findings[i], r.Findings[j]) })
- if fs := testFindings(r.Findings); !subset(test.want, fs) {
+ if fs := testFindings(allFindings(r)); !subset(test.want, fs) {
t.Errorf("want %v subset of findings; got %v", test.want, fs)
}
}
@@ -372,37 +378,8 @@
if err != nil {
t.Fatal(err)
}
- sort.SliceStable(r.Findings, func(i int, j int) bool { return audit.FindingCompare(r.Findings[i], r.Findings[j]) })
- if fs := testFindings(r.Findings); !subset(test.want, fs) {
+ if fs := testFindings(allFindings(r)); !subset(test.want, fs) {
t.Errorf("want %v subset of findings; got %v", test.want, fs)
}
}
}
-
-func vulnsToString(vulns []*osv.Entry) string {
- var s string
- for _, v := range vulns {
- s += fmt.Sprintf("\t%v\n", v)
- }
- return s
-}
-
-func TestUnreachable(t *testing.T) {
- r := &results{
- Vulns: []*osv.Entry{
- {ID: "0", Package: osv.Package{Name: "example.com/a"}, Affects: osv.Affects{Ranges: []osv.AffectsRange{{Type: osv.TypeSemver, Fixed: "1.0.0"}}}},
- {ID: "1", Package: osv.Package{Name: "example.com/b"}, Affects: osv.Affects{Ranges: []osv.AffectsRange{{Type: osv.TypeSemver, Fixed: "2.0.0"}}}},
- },
- Findings: []audit.Finding{
- {Vulns: []osv.Entry{{ID: "0"}}},
- },
- }
-
- expected := []*osv.Entry{
- {ID: "1", Package: osv.Package{Name: "example.com/b"}, Affects: osv.Affects{Ranges: []osv.AffectsRange{{Type: osv.TypeSemver, Fixed: "2.0.0"}}}},
- }
- unreachable := r.unreachable()
- if !reflect.DeepEqual(unreachable, expected) {
- t.Errorf("unreachable returned unexpected results: got\n%swant\n%s", vulnsToString(unreachable), vulnsToString(expected))
- }
-}
diff --git a/vulndb/internal/audit/detect.go b/vulndb/internal/audit/detect.go
index 8e1c0d0..c14e2c1 100644
--- a/vulndb/internal/audit/detect.go
+++ b/vulndb/internal/audit/detect.go
@@ -8,7 +8,7 @@
import (
"fmt"
"go/token"
- "io"
+ "sort"
"strings"
"golang.org/x/tools/go/packages"
@@ -17,13 +17,73 @@
// Preamble with types and common functionality used by vulnerability detection mechanisms in detect_*.go files.
+// SearchType represents a type of an audit search: call graph, imports, or binary.
+type SearchType int
+
+// enum values for SearchType.
+const (
+ CallGraphSearch SearchType = iota
+ ImportsSearch
+ BinarySearch
+)
+
+// Results contains the information on findings and identified vulnerabilities by audit search.
+type Results struct {
+ SearchMode SearchType
+
+ // TODO: identify vulnerability with <ID, package, symbol>?
+ // Vulnerabilities in dependent modules.
+ Vulnerabilities []osv.Entry
+
+ VulnFindings map[string][]Finding // vuln.ID -> findings
+}
+
+// String method for results.
+func (r Results) String() string {
+ sort.Slice(r.Vulnerabilities, func(i, j int) bool { return r.Vulnerabilities[i].ID < r.Vulnerabilities[j].ID })
+
+ rStr := ""
+ for _, v := range r.Vulnerabilities {
+ findings := r.VulnFindings[v.ID]
+ if len(findings) == 0 {
+ // TODO: add messages for such cases too?
+ continue
+ }
+
+ var alias string
+ if len(v.Aliases) == 0 {
+ alias = v.EcosystemSpecific.URL
+ } else {
+ alias = strings.Join(v.Aliases, ", ")
+ }
+ rStr += fmt.Sprintf("Findings for vulnerability: %s (of package %s):\n\n", alias, v.Package.Name)
+
+ for _, finding := range findings {
+ rStr += finding.String() + "\n"
+ }
+ }
+ return rStr
+}
+
+// addFindings adds a findings `f` for vulnerability `v`.
+func (r Results) addFinding(v osv.Entry, f Finding) {
+ r.VulnFindings[v.ID] = append(r.VulnFindings[v.ID], f)
+}
+
+// sort orders findings for each vulnerability based on its
+// perceived usefulness to the user.
+func (r Results) sort() {
+ for _, fs := range r.VulnFindings {
+ sort.SliceStable(fs, func(i int, j int) bool { return findingCompare(fs[i], fs[j]) })
+ }
+}
+
// Finding represents a finding for the use of a vulnerable symbol or an imported vulnerable package.
-// Provides info on symbol location, trace leading up to the symbol use, and associated vulnerabilities.
+// Provides info on symbol location and the trace leading up to the symbol use.
type Finding struct {
Symbol string
Position *token.Position `json:",omitempty"`
Type SymbolType
- Vulns []osv.Entry
Trace []TraceElem
// Approximate measure for indicating how useful the finding might be to the audit client.
@@ -31,6 +91,27 @@
weight int
}
+// String method for findings.
+func (f Finding) String() string {
+ traceStr := traceString(f.Trace)
+
+ var pos string
+ if f.Position != nil {
+ pos = fmt.Sprintf(" (%s)", f.Position)
+ }
+
+ return fmt.Sprintf("Trace:\n%s%s\n%s\n", f.Symbol, pos, traceStr)
+}
+
+func traceString(trace []TraceElem) string {
+ // traces are typically short, so string builders are not necessary
+ traceStr := ""
+ for i := len(trace) - 1; i >= 0; i-- {
+ traceStr += trace[i].String() + "\n"
+ }
+ return traceStr
+}
+
// SymbolType represents a type of a symbol use: function, global, or an import statement.
type SymbolType int
@@ -47,40 +128,12 @@
Position *token.Position `json:",omitempty"`
}
-// Write method for findings showing the trace and the associated vulnerabilities.
-func (f Finding) Write(w io.Writer) {
- var pos string
- if f.Position != nil {
- pos = fmt.Sprintf(" (%s)", f.Position)
+// String method for trace elements.
+func (e TraceElem) String() string {
+ if e.Position == nil {
+ return fmt.Sprintf("%s", e.Description)
}
- fmt.Fprintf(w, "Trace:\n%s%s\n", f.Symbol, pos)
- writeTrace(w, f.Trace)
- io.WriteString(w, "\n")
- writeVulns(w, f.Vulns)
- io.WriteString(w, "\n")
-}
-
-// writeTrace in reverse order, e.g., entry point is written last.
-func writeTrace(w io.Writer, trace []TraceElem) {
- for i := len(trace) - 1; i >= 0; i-- {
- trace[i].Write(w)
- io.WriteString(w, "\n")
- }
-}
-
-func writeVulns(w io.Writer, vulns []osv.Entry) {
- fmt.Fprintf(w, "Vulnerabilities:\n")
- for _, v := range vulns {
- fmt.Fprintf(w, "%s (%s)\n", v.Package.Name, v.EcosystemSpecific.URL)
- }
-}
-
-func (e TraceElem) Write(w io.Writer) {
- var pos string
- if e.Position != nil {
- pos = fmt.Sprintf(" (%s)", e.Position)
- }
- fmt.Fprintf(w, "%s%s", e.Description, pos)
+ return fmt.Sprintf("%s (%s)", e.Description, e.Position)
}
// MarshalText implements the encoding.TextMarshaler interface.
@@ -163,7 +216,7 @@
}
// VulnsForPackage returns the vulnerabilities for the module which is the most
-// specific prefixof importPath, or nil if there is no matching module with
+// specific prefix of importPath, or nil if there is no matching module with
// vulnerabilities.
func (mv ModuleVulnerabilities) VulnsForPackage(importPath string) []*osv.Entry {
var mostSpecificMod *modVulns
@@ -193,6 +246,7 @@
return packageVulns
}
+// VulnsForSymbol returns vulnerabilites for `symbol` in `mv.VulnsForPackage(importPath)`.
func (mv ModuleVulnerabilities) VulnsForSymbol(importPath, symbol string) []*osv.Entry {
vulns := mv.VulnsForPackage(importPath)
if vulns == nil {
@@ -214,3 +268,18 @@
}
return symbolVulns
}
+
+// Vulns returns vulnerabilities for all modules in `mv`.
+func (mv ModuleVulnerabilities) Vulns() []*osv.Entry {
+ var vulns []*osv.Entry
+ seen := make(map[string]bool)
+ for _, mv := range mv {
+ for _, v := range mv.vulns {
+ if !seen[v.ID] {
+ vulns = append(vulns, v)
+ seen[v.ID] = true
+ }
+ }
+ }
+ return vulns
+}
diff --git a/vulndb/internal/audit/detect_binary.go b/vulndb/internal/audit/detect_binary.go
index b9bf49d..b8e2a9b 100644
--- a/vulndb/internal/audit/detect_binary.go
+++ b/vulndb/internal/audit/detect_binary.go
@@ -9,23 +9,32 @@
)
// VulnerablePackageSymbols returns a list of vulnerability findings for per-package symbols
-// in packageSymbols, given the vulnerability and platform info captured in env.
+// in packageSymbols, given the `modVulns` vulnerabilities.
//
-// Returned Findings only have Symbol, Type, and Vulns fields set.
-func VulnerablePackageSymbols(packageSymbols map[string][]string, modVulns ModuleVulnerabilities) []Finding {
- var findings []Finding
+// Findings for each vulnerability are sorted by estimated usefulness to the user and do not
+// have an associated trace.
+func VulnerablePackageSymbols(packageSymbols map[string][]string, modVulns ModuleVulnerabilities) Results {
+ results := Results{
+ SearchMode: BinarySearch,
+ Vulnerabilities: serialize(modVulns.Vulns()),
+ VulnFindings: make(map[string][]Finding),
+ }
+ if len(modVulns) == 0 {
+ return results
+ }
+
for pkg, symbols := range packageSymbols {
for _, symbol := range symbols {
- if vulns := modVulns.VulnsForSymbol(pkg, symbol); len(vulns) > 0 {
- findings = append(findings,
- Finding{
- Symbol: fmt.Sprintf("%s.%s", pkg, symbol),
- Type: GlobalType,
- Vulns: serialize(vulns),
- })
+ vulns := modVulns.VulnsForSymbol(pkg, symbol)
+ for _, v := range serialize(vulns) {
+ results.addFinding(v, Finding{
+ Symbol: fmt.Sprintf("%s.%s", pkg, symbol),
+ Type: GlobalType,
+ })
}
}
}
- return findings
+ results.sort()
+ return results
}
diff --git a/vulndb/internal/audit/detect_callgraph.go b/vulndb/internal/audit/detect_callgraph.go
index 34e434d..148cd63 100644
--- a/vulndb/internal/audit/detect_callgraph.go
+++ b/vulndb/internal/audit/detect_callgraph.go
@@ -18,9 +18,9 @@
"golang.org/x/tools/go/callgraph/vta"
)
-// VulnerableSymbols returns a list of vulnerability findings for symbols transitively reachable
-// through the callgraph built using VTA analysis from the entry points of pkgs, given the
-// vulnerability and platform info captured in env.
+// VulnerableSymbols returns vulnerability findings for symbols transitively reachable
+// through the callgraph built using VTA analysis from the entry points of pkgs, given
+// the vulnerability and platform info captured in env.
//
// Returns all findings reachable from pkgs while analyzing each package only once, prefering findings
// of shorter import traces. For instance, given call chains
@@ -33,8 +33,19 @@
// D() -> B() -> V
// as traces of transitively using a vulnerable symbol V.
//
+// Findings for each vulnerability are sorted by estimated usefulness to the user.
+//
// Panics if packages in pkgs do not belong to the same program.
-func VulnerableSymbols(pkgs []*ssa.Package, modVulns ModuleVulnerabilities) []Finding {
+func VulnerableSymbols(pkgs []*ssa.Package, modVulns ModuleVulnerabilities) Results {
+ results := Results{
+ SearchMode: CallGraphSearch,
+ Vulnerabilities: serialize(modVulns.Vulns()),
+ VulnFindings: make(map[string][]Finding),
+ }
+ if len(modVulns) == 0 {
+ return results
+ }
+
prog := pkgsProgram(pkgs)
if prog == nil {
panic("packages in pkgs must belong to a single common program")
@@ -47,7 +58,6 @@
queue.PushBack(&callChain{f: entry})
}
- var findings []Finding
seen := make(map[*ssa.Function]bool)
for queue.Len() > 0 {
front := queue.Front()
@@ -59,14 +69,14 @@
}
seen[v.f] = true
- finds, calls := funcVulnsAndCalls(v, modVulns, callGraph)
- findings = append(findings, finds...)
+ calls := funcVulnsAndCalls(v, modVulns, &results, callGraph)
for _, call := range calls {
queue.PushBack(call)
}
}
- return findings
+ results.sort()
+ return results
}
// callGraph builds a call graph of prog based on VTA analysis.
@@ -175,15 +185,14 @@
return callWeight + chain.parent.weight()
}
-// funcVulnsAndCalls returns a list of symbol findings for function at the top
-// of chain and next calls to analyze.
-func funcVulnsAndCalls(chain *callChain, modVulns ModuleVulnerabilities, callGraph *callgraph.Graph) ([]Finding, []*callChain) {
- var findings []Finding
+// funcVulnsAndCalls adds symbol findings to results for
+// function at the top of chain and next calls to analyze.
+func funcVulnsAndCalls(chain *callChain, modVulns ModuleVulnerabilities, results *Results, callGraph *callgraph.Graph) []*callChain {
var calls []*callChain
for _, b := range chain.f.Blocks {
for _, instr := range b.Instrs {
// First collect all findings for globals except callees in function call statements.
- findings = append(findings, globalFindings(globalUses(instr), chain, modVulns)...)
+ globalFindings(globalUses(instr), chain, modVulns, results)
// Callees are handled separately to produce call findings rather than global findings.
site, ok := instr.(ssa.CallInstruction)
@@ -191,78 +200,68 @@
continue
}
- callees := siteCallees(site, callGraph)
- for _, callee := range callees {
+ for _, callee := range siteCallees(site, callGraph) {
c := &callChain{call: site, f: callee, parent: chain}
calls = append(calls, c)
-
- if f := callFinding(c, modVulns); f != nil {
- findings = append(findings, *f)
- }
+ callFinding(c, modVulns, results)
}
}
}
- return findings, calls
+ return calls
}
-// globalFindings returns findings for vulnerable globals among globalUses.
+// globalFindings adds findings for vulnerable globals among globalUses to results.
// Assumes each use in globalUses is a use of a global variable. Can generate
// duplicates when globalUses contains duplicates.
-func globalFindings(globalUses []*ssa.Value, chain *callChain, modVulns ModuleVulnerabilities) []Finding {
+func globalFindings(globalUses []*ssa.Value, chain *callChain, modVulns ModuleVulnerabilities, results *Results) {
if underRelatedVuln(chain, modVulns) {
- return nil
+ return
}
- var findings []Finding
for _, o := range globalUses {
g := (*o).(*ssa.Global)
vulns := modVulns.VulnsForSymbol(g.Package().Pkg.Path(), g.Name())
- if len(vulns) > 0 {
- findings = append(findings,
- Finding{
- Symbol: fmt.Sprintf("%s.%s", g.Package().Pkg.Path(), g.Name()),
- Trace: chain.trace(),
- Position: valPosition(*o, chain.f),
- Type: GlobalType,
- Vulns: serialize(vulns),
- weight: chain.weight()})
+ for _, v := range serialize(vulns) {
+ results.addFinding(v, Finding{
+ Symbol: fmt.Sprintf("%s.%s", g.Package().Pkg.Path(), g.Name()),
+ Trace: chain.trace(),
+ Position: valPosition(*o, chain.f),
+ Type: GlobalType,
+ weight: chain.weight()})
}
}
- return findings
}
-// callFinding returns vulnerability finding for the call made at the top of the chain.
+// callFinding adds findings to results for the call made at the top of the chain.
// If there is no vulnerability or no call information, then nil is returned.
// TODO(zpavlinovic): remove ssa info from higher-order calls.
-func callFinding(chain *callChain, modVulns ModuleVulnerabilities) *Finding {
+func callFinding(chain *callChain, modVulns ModuleVulnerabilities, results *Results) {
if underRelatedVuln(chain, modVulns) {
- return nil
+ return
}
callee := chain.f
call := chain.call
if callee == nil || call == nil {
- return nil
+ return
+ }
+
+ c := chain
+ if !unresolved(call) {
+ // If the last call is a resolved callsite, remove the edge from the trace as that
+ // information is provided in the symbol field.
+ c = c.parent
}
vulns := modVulns.VulnsForSymbol(callee.Package().Pkg.Path(), dbFuncName(callee))
- if len(vulns) > 0 {
- c := chain
- if !unresolved(call) {
- // If the last call is a resolved callsite, remove the edge from the trace as that
- // information is provided in the symbol field.
- c = c.parent
- }
- return &Finding{
+ for _, v := range serialize(vulns) {
+ results.addFinding(v, Finding{
Symbol: fmt.Sprintf("%s.%s", callee.Package().Pkg.Path(), dbFuncName(callee)),
Trace: c.trace(),
Position: instrPosition(call),
Type: FunctionType,
- Vulns: serialize(vulns),
- weight: c.weight()}
+ weight: c.weight()})
}
-
- return nil
}
// Checks if a potential vulnerability in chain.f is analyzed only because
diff --git a/vulndb/internal/audit/detect_callgraph_test.go b/vulndb/internal/audit/detect_callgraph_test.go
index 615e1fd..4813770 100644
--- a/vulndb/internal/audit/detect_callgraph_test.go
+++ b/vulndb/internal/audit/detect_callgraph_test.go
@@ -7,74 +7,76 @@
import (
"go/token"
"reflect"
- "sort"
"testing"
-
- "golang.org/x/vulndb/osv"
)
func TestSymbolVulnDetectionVTA(t *testing.T) {
pkgs, modVulns := testContext(t)
- got := projectFindings(VulnerableSymbols(pkgs, modVulns))
+ results := VulnerableSymbols(pkgs, modVulns)
- // There should be four call chains reported with VTA-VTA version, in the following order:
+ if results.SearchMode != CallGraphSearch {
+ t.Errorf("want call graph search mode; got %v", results.SearchMode)
+ }
+
+ // There should be four call chains reported with VTA-VTA version, in the following order,
+ // for vuln.VG and vuln.VulnData.Vuln vulnerabilities:
+ // vuln.VG:
// T:T1() -> vuln.VG [use of global at line 4]
- // T:T1() -> A:A1() -> vuln.VulnData.Vuln() [call at A.go:14]
// T:T2() -> vuln.Vuln() [approx.resolved] -> vuln.VG [use of global at vuln.go:4]
+ // vuln.VulnData.Vuln:
+ // T:T1() -> A:A1() -> vuln.VulnData.Vuln() [call at A.go:14]
// T:T1() -> vuln.VulnData.Vuln() [approx. resolved] [call at testdata.go:13]
// Without VTA-VTA, we would alse have the following false positive:
// T:T2() -> vuln.VulnData.Vuln() [approx. resolved] [call at testdata.go:26]
- want := []Finding{
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln.VG",
- Trace: []TraceElem{
- {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
+ for _, test := range []struct {
+ vulnId string
+ findings []Finding
+ }{
+ {vulnId: "V1", findings: []Finding{
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln.VulnData.Vuln",
+ Trace: []TraceElem{
+ {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
+ {Description: "a.org/A.A1(...)", Position: &token.Position{Line: 14, Filename: "T.go"}}},
+ Type: FunctionType,
+ Position: &token.Position{Line: 15, Filename: "A.go"},
+ weight: 0,
},
- Type: GlobalType,
- Position: &token.Position{Line: 5, Filename: "vuln.go"},
- Vulns: []osv.Entry{{Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 0,
- },
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln.VulnData.Vuln",
- Trace: []TraceElem{
- {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
- {Description: "a.org/A.A1(...)", Position: &token.Position{Line: 14, Filename: "T.go"}}},
- Type: FunctionType,
- Position: &token.Position{Line: 15, Filename: "A.go"},
- Vulns: []osv.Entry{{Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 0,
- },
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln.VG",
- Trace: []TraceElem{
- {Description: "command-line-arguments.T2(...)", Position: &token.Position{Line: 20, Filename: "T.go"}},
- {Description: "command-line-arguments.t0(...) [approx. resolved to thirdparty.org/vulnerabilities/vuln.Vuln]", Position: &token.Position{Line: 22, Filename: "T.go"}},
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln.VulnData.Vuln",
+ Trace: []TraceElem{
+ {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
+ {Description: "a.org/A.I.Vuln(...) [approx. resolved to (thirdparty.org/vulnerabilities/vuln.VulnData).Vuln]", Position: &token.Position{Line: 14, Filename: "T.go"}}},
+ Type: FunctionType,
+ Position: &token.Position{Line: 14, Filename: "T.go"},
+ weight: 1,
},
- Type: GlobalType,
- Position: &token.Position{Line: 5, Filename: "vuln.go"},
- Vulns: []osv.Entry{{Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 1,
- },
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln.VulnData.Vuln",
- Trace: []TraceElem{
- {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
- {Description: "a.org/A.I.Vuln(...) [approx. resolved to (thirdparty.org/vulnerabilities/vuln.VulnData).Vuln]", Position: &token.Position{Line: 14, Filename: "T.go"}}},
- Type: FunctionType,
- Position: &token.Position{Line: 14, Filename: "T.go"},
- Vulns: []osv.Entry{{Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 1,
- },
- }
-
- if len(want) != len(got) {
- t.Errorf("want %d findings; got %d", len(want), len(got))
- return
- }
-
- sort.SliceStable(got, func(i int, j int) bool { return FindingCompare(got[i], got[j]) })
- if !reflect.DeepEqual(want, got) {
- t.Errorf("want %v findings (projected); got %v", want, got)
+ }},
+ {vulnId: "V2", findings: []Finding{
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln.VG",
+ Trace: []TraceElem{
+ {Description: "command-line-arguments.T1(...)", Position: &token.Position{Line: 11, Filename: "T.go"}},
+ },
+ Type: GlobalType,
+ Position: &token.Position{Line: 5, Filename: "vuln.go"},
+ weight: 0,
+ },
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln.VG",
+ Trace: []TraceElem{
+ {Description: "command-line-arguments.T2(...)", Position: &token.Position{Line: 20, Filename: "T.go"}},
+ {Description: "command-line-arguments.t0(...) [approx. resolved to thirdparty.org/vulnerabilities/vuln.Vuln]", Position: &token.Position{Line: 22, Filename: "T.go"}},
+ },
+ Type: GlobalType,
+ Position: &token.Position{Line: 5, Filename: "vuln.go"},
+ weight: 1,
+ },
+ }},
+ } {
+ got := projectFindings(results.VulnFindings[test.vulnId])
+ if !reflect.DeepEqual(test.findings, got) {
+ t.Errorf("want %v findings (projected); got %v", test.findings, got)
+ }
}
}
diff --git a/vulndb/internal/audit/detect_imports.go b/vulndb/internal/audit/detect_imports.go
index f10de30..41f723d 100644
--- a/vulndb/internal/audit/detect_imports.go
+++ b/vulndb/internal/audit/detect_imports.go
@@ -11,7 +11,7 @@
"golang.org/x/tools/go/ssa"
)
-// VulnerableImports returns a list of vulnerability findings for packages imported by `pkgs`
+// VulnerableImports returns vulnerability findings for packages imported by `pkgs`
// given the vulnerability and platform info captured in `env`.
//
// Returns all findings reachable from `pkgs` while analyzing each package only once, prefering
@@ -24,8 +24,18 @@
// or
// D -> B -> V
// as traces of importing a vulnerable package V.
-func VulnerableImports(pkgs []*ssa.Package, modVulns ModuleVulnerabilities) []Finding {
- var findings []Finding
+//
+// Findings for each vulnerability are sorted by estimated usefulness to the user.
+func VulnerableImports(pkgs []*ssa.Package, modVulns ModuleVulnerabilities) Results {
+ results := Results{
+ SearchMode: ImportsSearch,
+ Vulnerabilities: serialize(modVulns.Vulns()),
+ VulnFindings: make(map[string][]Finding),
+ }
+ if len(modVulns) == 0 {
+ return results
+ }
+
seen := make(map[string]bool)
queue := list.New()
for _, pkg := range pkgs {
@@ -34,10 +44,10 @@
for queue.Len() > 0 {
front := queue.Front()
- v := front.Value.(*importChain)
+ c := front.Value.(*importChain)
queue.Remove(front)
- pkg := v.pkg
+ pkg := c.pkg
if pkg == nil {
continue
}
@@ -49,20 +59,19 @@
for _, imp := range pkg.Imports() {
vulns := modVulns.VulnsForPackage(imp.Path())
- if len(vulns) > 0 {
- findings = append(findings,
- Finding{
- Symbol: imp.Path(),
- Type: ImportType,
- Trace: v.trace(),
- Vulns: serialize(vulns),
- weight: len(v.trace())})
+ for _, v := range serialize(vulns) {
+ results.addFinding(v, Finding{
+ Symbol: imp.Path(),
+ Type: ImportType,
+ Trace: c.trace(),
+ weight: len(c.trace())})
}
- queue.PushBack(&importChain{pkg: imp, parent: v})
+ queue.PushBack(&importChain{pkg: imp, parent: c})
}
}
- return findings
+ results.sort()
+ return results
}
// importChain helps doing BFS over package imports while remembering import chains.
diff --git a/vulndb/internal/audit/detect_imports_test.go b/vulndb/internal/audit/detect_imports_test.go
index e36f73a..e0f3968 100644
--- a/vulndb/internal/audit/detect_imports_test.go
+++ b/vulndb/internal/audit/detect_imports_test.go
@@ -6,47 +6,57 @@
import (
"reflect"
- "sort"
"testing"
-
- "golang.org/x/vulndb/osv"
)
func TestImportedPackageVulnDetection(t *testing.T) {
pkgs, modVulns := testContext(t)
- got := projectFindings(VulnerableImports(pkgs, modVulns))
+ results := VulnerableImports(pkgs, modVulns)
- // There should be two chains reported in the following order:
+ if results.SearchMode != ImportsSearch {
+ t.Errorf("want import search mode; got %v", results.SearchMode)
+ }
+
+ // There should be two chains reported in the following order
+ // for two of the thirdparty.org test vulnerabilities:
// T -> vuln
// T -> A -> vuln
- want := []Finding{
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln",
- Trace: []TraceElem{{Description: "command-line-arguments"}},
- Type: ImportType,
- Vulns: []osv.Entry{
- {Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}},
- {Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 1,
- },
- {
- Symbol: "thirdparty.org/vulnerabilities/vuln",
- Trace: []TraceElem{{Description: "command-line-arguments"}, {Description: "a.org/A"}},
- Type: ImportType,
- Vulns: []osv.Entry{
- {Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}},
- {Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"}}},
- weight: 2,
- },
- }
-
- if len(want) != len(got) {
- t.Errorf("want %d findings; got %d", len(want), len(got))
- return
- }
-
- sort.SliceStable(got, func(i int, j int) bool { return FindingCompare(got[i], got[j]) })
- if !reflect.DeepEqual(want, got) {
- t.Errorf("want %v findings (projected); got %v", want, got)
+ for _, test := range []struct {
+ vulnId string
+ findings []Finding
+ }{
+ {vulnId: "V1", findings: []Finding{
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln",
+ Trace: []TraceElem{{Description: "command-line-arguments"}},
+ Type: ImportType,
+ weight: 1,
+ },
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln",
+ Trace: []TraceElem{{Description: "command-line-arguments"}, {Description: "a.org/A"}},
+ Type: ImportType,
+ weight: 2,
+ },
+ }},
+ {vulnId: "V2", findings: []Finding{
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln",
+ Trace: []TraceElem{{Description: "command-line-arguments"}},
+ Type: ImportType,
+ weight: 1,
+ },
+ {
+ Symbol: "thirdparty.org/vulnerabilities/vuln",
+ Trace: []TraceElem{{Description: "command-line-arguments"}, {Description: "a.org/A"}},
+ Type: ImportType,
+ weight: 2,
+ },
+ }},
+ } {
+ got := projectFindings(results.VulnFindings[test.vulnId])
+ if !reflect.DeepEqual(test.findings, got) {
+ t.Errorf("want %v findings (projected); got %v", test.findings, got)
+ }
}
}
diff --git a/vulndb/internal/audit/helpers_test.go b/vulndb/internal/audit/helpers_test.go
index a93086d..8609f00 100644
--- a/vulndb/internal/audit/helpers_test.go
+++ b/vulndb/internal/audit/helpers_test.go
@@ -73,11 +73,13 @@
mod: &packages.Module{Path: "thirdparty.org/vulnerabilities", Version: "v1.0.1"},
vulns: []*osv.Entry{
{
+ ID: "V1",
Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"},
Affects: osv.Affects{Ranges: []osv.AffectsRange{{Type: osv.TypeSemver, Introduced: "1.0.0", Fixed: "1.0.4"}, {Type: osv.TypeSemver, Introduced: "1.1.2"}}},
EcosystemSpecific: osv.GoSpecific{Symbols: []string{"VulnData.Vuln", "VulnData.VulnOnPtr"}},
},
{
+ ID: "V2",
Package: osv.Package{Name: "thirdparty.org/vulnerabilities/vuln"},
Affects: osv.Affects{Ranges: []osv.AffectsRange{{Type: osv.TypeSemver, Introduced: "1.0.1", Fixed: "1.0.2"}}},
EcosystemSpecific: osv.GoSpecific{Symbols: []string{"VG"}},
@@ -146,7 +148,6 @@
Symbol: f.Symbol,
Position: projectPosition(f.Position),
Trace: projectTrace(f.Trace),
- Vulns: projectVulns(f.Vulns),
weight: f.weight,
}
nfs = append(nfs, nf)
diff --git a/vulndb/internal/audit/order.go b/vulndb/internal/audit/order.go
index 0f595f7..5161bf2 100644
--- a/vulndb/internal/audit/order.go
+++ b/vulndb/internal/audit/order.go
@@ -9,10 +9,10 @@
"strings"
)
-// FindingCompare compares two findings in terms of their approximate usefulness to the user.
+// findingCompare compares two findings in terms of their approximate usefulness to the user.
// A finding that either has 1) shorter trace, or 2) less unresolved call sites in the trace
// is considered smaller, i.e., better.
-func FindingCompare(finding1, finding2 Finding) bool {
+func findingCompare(finding1, finding2 Finding) bool {
if len(finding1.Trace) < len(finding2.Trace) {
return true
} else if len(finding2.Trace) < len(finding1.Trace) {
