diff --git a/cmd/vulnreport/find_aliases.go b/cmd/vulnreport/find_aliases.go index 3e7faae5..f3497038 100644 --- a/cmd/vulnreport/find_aliases.go +++ b/cmd/vulnreport/find_aliases.go @@ -5,6 +5,7 @@ package main import ( + "bytes" "context" "fmt" @@ -172,19 +173,31 @@ type ghsaClient interface { type memGC struct { ghsas map[string]ghsa.SecurityAdvisory + byCVE map[string][]*ghsa.SecurityAdvisory } func newMemGC(archive []byte) (*memGC, error) { ar := txtar.Parse(archive) m := &memGC{ ghsas: make(map[string]ghsa.SecurityAdvisory), + byCVE: make(map[string][]*ghsa.SecurityAdvisory), } for _, f := range ar.Files { var g ghsa.SecurityAdvisory - if err := yaml.Unmarshal(f.Data, &g); err != nil { + d := yaml.NewDecoder(bytes.NewBuffer(f.Data)) + d.KnownFields(true) + if err := d.Decode(&g); err != nil { return nil, err } + if f.Name != g.ID { + return nil, fmt.Errorf("filename (%s) != ID (%s)", f.Name, g.ID) + } m.ghsas[f.Name] = g + for _, id := range g.Identifiers { + if id.Type == "CVE" { + m.byCVE[id.Value] = append(m.byCVE[id.Value], &g) + } + } } return m, nil } @@ -197,12 +210,5 @@ func (m *memGC) FetchGHSA(_ context.Context, id string) (*ghsa.SecurityAdvisory, } func (m *memGC) ListForCVE(_ context.Context, cid string) (result []*ghsa.SecurityAdvisory, _ error) { - for _, sa := range m.ghsas { - for _, id := range sa.Identifiers { - if id.Type == "CVE" || id.Value == cid { - result = append(result, &sa) - } - } - } - return result, nil + return m.byCVE[cid], nil } diff --git a/cmd/vulnreport/lint.go b/cmd/vulnreport/lint.go index 87690e43..f2c7862b 100644 --- a/cmd/vulnreport/lint.go +++ b/cmd/vulnreport/lint.go @@ -55,10 +55,10 @@ func (l *linter) lint(r *yamlReport) error { } func (l *linter) canonicalModule(mp string) string { - if module, err := l.pxc.FindModule(mp); err == nil { // no error + if module, err := l.pxc.FindModule(mp); module != "" && err == nil { // no error mp = module } - if module, err := l.pxc.CanonicalAtLatest(mp); err == nil { // no error + if module, err := l.pxc.CanonicalAtLatest(mp); module != "" && err == nil { // no error mp = module } return mp diff --git a/cmd/vulnreport/run_test.go b/cmd/vulnreport/run_test.go index f9dd6dec..143bea07 100644 --- a/cmd/vulnreport/run_test.go +++ b/cmd/vulnreport/run_test.go @@ -12,11 +12,13 @@ import ( "fmt" "io/fs" "path/filepath" + "slices" "strings" "testing" "time" "github.com/google/go-cmp/cmp" + "golang.org/x/exp/maps" "golang.org/x/tools/txtar" "golang.org/x/vulndb/cmd/vulnreport/log" "golang.org/x/vulndb/cmd/vulnreport/priority" @@ -206,8 +208,10 @@ func writeGolden(t *testing.T, g *golden, comment string, written map[string][]b {Name: "out", Data: g.out}, {Name: "logs", Data: g.logs}, } - for fname, b := range written { - files = append(files, txtar.File{Name: fname, Data: b}) + sortedFilenames := maps.Keys(written) + slices.Sort(sortedFilenames) + for _, fname := range sortedFilenames { + files = append(files, txtar.File{Name: fname, Data: written[fname]}) } return test.WriteTxtar(testFilename(t), files, comment) diff --git a/cmd/vulnreport/testdata/TestFix/no_change.txtar b/cmd/vulnreport/testdata/TestFix/no_change.txtar index 0ee6ba1c..a8984bc5 100644 --- a/cmd/vulnreport/testdata/TestFix/no_change.txtar +++ b/cmd/vulnreport/testdata/TestFix/no_change.txtar @@ -17,16 +17,6 @@ info: GO-9999-0001: skipping symbol checks for package golang.org/x/vulndb/cmd/v info: GO-9999-0001: checking for missing GHSAs and CVEs (use -skip-alias to skip this) info: GO-9999-0001: checking that all references are reachable info: fix: processed 1 report(s) (success=1; skip=0; error=0) --- data/reports/GO-9999-0001.yaml -- -id: GO-9999-0001 -modules: - - module: golang.org/x/vulndb - vulnerable_at: 0.0.0-20240716161253-dd7900b89e20 - packages: - - package: golang.org/x/vulndb/cmd/vulnreport -summary: A problem with golang.org/x/vulndb -description: A description of the issue -review_status: REVIEWED -- data/osv/GO-9999-0001.json -- { "schema_version": "1.3.1", @@ -65,3 +55,13 @@ review_status: REVIEWED "review_status": "REVIEWED" } } +-- data/reports/GO-9999-0001.yaml -- +id: GO-9999-0001 +modules: + - module: golang.org/x/vulndb + vulnerable_at: 0.0.0-20240716161253-dd7900b89e20 + packages: + - package: golang.org/x/vulndb/cmd/vulnreport +summary: A problem with golang.org/x/vulndb +description: A description of the issue +review_status: REVIEWED diff --git a/cmd/vulnreport/testdata/TestTriage/all.txtar b/cmd/vulnreport/testdata/TestTriage/all.txtar index 6c94c064..c4de722e 100644 --- a/cmd/vulnreport/testdata/TestTriage/all.txtar +++ b/cmd/vulnreport/testdata/TestTriage/all.txtar @@ -12,17 +12,22 @@ issue test-issue-tracker/10 is high priority - golang.org/x/vuln has 101 importers (>= 100) and as many reviewed (0) as likely-binary reports (0) issue test-issue-tracker/11 is possibly not Go - more than 20 percent of reports (1 of 1) with this module are NOT_GO_CODE -triaged 3 issues: +issue test-issue-tracker/12 is likely duplicate + - #12 shares alias(es) CVE-1999-0002, GHSA-xxxx-yyyy-0002, GHSA-xxxx-yyyy-0003 with test-issue-tracker/13 + - #12 shares alias(es) CVE-1999-0002, GHSA-xxxx-yyyy-0002, GHSA-xxxx-yyyy-0003 with test-issue-tracker/14 +issue test-issue-tracker/13 is likely duplicate + - #13 shares alias(es) CVE-1999-0002, GHSA-xxxx-yyyy-0002, GHSA-xxxx-yyyy-0003 with test-issue-tracker/14 +triaged 6 issues: - 1 high priority - - 2 low priority + - 5 low priority - 0 unknown priority - - 1 likely duplicate + - 3 likely duplicate - 1 possibly not Go helpful commands: $ vulnreport create 10 -- logs -- info: creating alias map for open issues -info: triage: operating on 4 issue(s) +info: triage: operating on 7 issue(s) info: triage: skipping issue #1 (already has report) info: triage 7 info: issue test-issue-tracker/7 is low priority @@ -31,4 +36,13 @@ info: triage 10 info: triage 11 info: issue test-issue-tracker/11 is low priority - collectd.org has 0 importers (< 100) -info: triage: processed 4 issue(s) (success=3; skip=1; error=0) +info: triage 12 +info: issue test-issue-tracker/12 is low priority + - golang.org/x/tools has 50 importers (< 100) +info: triage 13 +info: issue test-issue-tracker/13 is low priority + - golang.org/x/tools has 50 importers (< 100) +info: triage 14 +info: issue test-issue-tracker/14 is low priority + - golang.org/x/tools has 50 importers (< 100) +info: triage: processed 7 issue(s) (success=6; skip=1; error=0) diff --git a/cmd/vulnreport/testdata/issue_tracker.txtar b/cmd/vulnreport/testdata/issue_tracker.txtar index fb523c26..4c515de2 100644 --- a/cmd/vulnreport/testdata/issue_tracker.txtar +++ b/cmd/vulnreport/testdata/issue_tracker.txtar @@ -26,3 +26,18 @@ number: 11 title: "x/vulndb: potential Go vuln in collectd.org: CVE-2021-0000" state: open +-- 12 -- +number: 12 +title: "x/vulndb: potential Go vuln in golang.org/x/tools: GHSA-xxxx-yyyy-0002" +state: open + +-- 13 -- +number: 13 +title: "x/vulndb: potential Go vuln in golang.org/x/tools: CVE-1999-0002" +state: open + +-- 14 -- +number: 14 +title: "x/vulndb: potential Go vuln in golang.org/x/tools: GHSA-xxxx-yyyy-0003" +state: open + diff --git a/cmd/vulnreport/testdata/legacy_ghsas.txtar b/cmd/vulnreport/testdata/legacy_ghsas.txtar index a7635b40..c3961e70 100644 --- a/cmd/vulnreport/testdata/legacy_ghsas.txtar +++ b/cmd/vulnreport/testdata/legacy_ghsas.txtar @@ -7,9 +7,20 @@ -- GHSA-xxxx-yyyy-zzzz -- id: GHSA-xxxx-yyyy-zzzz identifiers: - - identifier: - - type: CVE - id: CVE-1999-0005 + - type: CVE + value: CVE-1999-0005 -- GHSA-xxxx-yyyy-0001 -- id: GHSA-xxxx-yyyy-0001 + +-- GHSA-xxxx-yyyy-0002 -- +id: GHSA-xxxx-yyyy-0002 +identifiers: + - type: CVE + value: CVE-1999-0002 + +-- GHSA-xxxx-yyyy-0003 -- +id: GHSA-xxxx-yyyy-0003 +identifiers: + - type: CVE + value: CVE-1999-0002 diff --git a/cmd/vulnreport/triage.go b/cmd/vulnreport/triage.go index 202160c2..fb85a0ca 100644 --- a/cmd/vulnreport/triage.go +++ b/cmd/vulnreport/triage.go @@ -27,6 +27,9 @@ type triage struct { mu sync.Mutex // protects aliasesToIssues and stats aliasesToIssues map[string][]int stats []issuesList + + // issues that have already been marked as duplicate + duplicates map[int]bool } func (*triage) name() string { return "triage" } @@ -65,6 +68,7 @@ func (t *triage) setup(ctx context.Context, env environment) error { } log.Info("creating alias map for open issues") + t.duplicates = make(map[int]bool) open, err := t.openIssues(ctx) if err != nil { return err @@ -74,6 +78,9 @@ func (t *triage) setup(ctx context.Context, env environment) error { for _, a := range aliases { t.addAlias(a, iss.Number) } + if iss.HasLabel(labelPossibleDuplicate) || iss.HasLabel(labelDuplicate) { + t.duplicates[iss.Number] = true + } } return nil } @@ -127,6 +134,7 @@ func (t *triage) triage(ctx context.Context, iss *issues.Issue) { strings.Join(aliases, ", "), filepath.ToSlash(ref))) } + slices.Sort(strs) t.addStat(iss, statDuplicate, strings.Join(strs, listItem)) labels = append(labels, labelPossibleDuplicate) } @@ -191,8 +199,14 @@ func (t *triage) findDuplicates(ctx context.Context, iss *issues.Issue) map[stri if iss.Number == dup { continue } + // If the other issue is already marked as a duplicate, + // we don't need to mark this one. + if t.duplicates[dup] { + continue + } ref := t.ic.Reference(dup) xrefs[ref] = append(xrefs[ref], a) + t.duplicates[iss.Number] = true } }