vuln: reports can have multiple CVEs
A report can be associated with multiple CVEs.
The osv.Entry type already accommodated this, but the report.Report
type did not, so added Report.CVEs.
For backwards compatibility and easy of writing reports, leave
Report.CVE. The linter checks that at most one of CVE and CVEs is
populated.
Along the way, correct minor issues with style and doc.
Change-Id: Ib28aa4f022f9e01c2cfdf17a1d69dcac6a71b8dc
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/371596
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/cmd/gendb/main.go b/cmd/gendb/main.go
index 3bfad88..5282f4e 100644
--- a/cmd/gendb/main.go
+++ b/cmd/gendb/main.go
@@ -56,7 +56,7 @@
failf("unable to unmarshal %q: %s", f.Name(), err)
}
if lints := vuln.Lint(); len(lints) > 0 {
- fmt.Fprintf(os.Stderr, "invalid vulnerability file %q:\n", os.Args[1])
+ fmt.Fprintf(os.Stderr, "invalid vulnerability file %q:\n", f.Name())
for _, lint := range lints {
fmt.Fprintf(os.Stderr, "\t%s\n", lint)
}
diff --git a/cmd/report2cve/main.go b/cmd/report2cve/main.go
index e5729b9..c9de47a 100644
--- a/cmd/report2cve/main.go
+++ b/cmd/report2cve/main.go
@@ -22,7 +22,7 @@
func fromReport(r *report.Report) (_ *cveschema.CVE, err error) {
defer derrors.Wrap(&err, "fromReport(r)")
- if r.CVE != "" {
+ if r.CVE != "" || len(r.CVEs) > 0 {
return nil, errors.New("report has CVE ID is wrong section (should be in cve_metadata for self-issued CVEs)")
}
if r.CVEMetadata == nil {
diff --git a/internal/report/lint.go b/internal/report/lint.go
index 62ea635..936ef2c 100644
--- a/internal/report/lint.go
+++ b/internal/report/lint.go
@@ -48,7 +48,7 @@
return versions, nil
}
-func getCanonicalModName(module string, version string) (_ string, err error) {
+func getCanonicalModName(module, version string) (_ string, err error) {
defer derrors.Wrap(&err, "getCanonicalModName(%q, %q)", module, version)
resp, err := http.Get(fmt.Sprintf("%s/%s/@v/%s.mod", proxyURL, module, version))
if err != nil {
@@ -139,23 +139,25 @@
var cveRegex = regexp.MustCompile(`^CVE-\d{4}-\d{4,}$`)
// Lint checks the content of a Report.
-// TODO: instead of returning a single error we may want to return a slice, so that
-// we aren't fixing one thing at a time. Similarly it might make sense to include
-// warnings or informational things alongside errors, especially during for use
-// during the triage process.
+// TODO: It might make sense to include warnings or informational things
+// alongside errors, especially during for use during the triage process.
func (vuln *Report) Lint() []string {
var issues []string
+ addIssue := func(iss string) {
+ issues = append(issues, iss)
+ }
+
var importPath string
if !vuln.Stdlib {
if vuln.Module == "" {
- issues = append(issues, "missing module")
+ addIssue("missing module")
}
if vuln.Module != "" && vuln.Package == vuln.Module {
- issues = append(issues, "package is redundant and can be removed")
+ addIssue("package is redundant and can be removed")
}
if vuln.Package != "" && !strings.HasPrefix(vuln.Package, vuln.Module) {
- issues = append(issues, "module must be a prefix of package")
+ addIssue("module must be a prefix of package")
}
if vuln.Package == "" {
importPath = vuln.Module
@@ -164,27 +166,27 @@
}
if vuln.Module != "" && importPath != "" {
if err := checkModVersions(vuln.Module, vuln.Versions); err != nil {
- issues = append(issues, err.Error())
+ addIssue(err.Error())
}
if err := module.CheckImportPath(importPath); err != nil {
- issues = append(issues, err.Error())
+ addIssue(err.Error())
}
}
} else if vuln.Package == "" {
- issues = append(issues, "missing package")
+ addIssue("missing package")
}
for _, additionalPackage := range vuln.AdditionalPackages {
var additionalImportPath string
if additionalPackage.Module == "" {
- issues = append(issues, "missing additional_package.module")
+ addIssue("missing additional_package.module")
}
if additionalPackage.Package == additionalPackage.Module {
- issues = append(issues, "package is redundant and can be removed")
+ addIssue("package is redundant and can be removed")
}
if additionalPackage.Package != "" && !strings.HasPrefix(additionalPackage.Package, additionalPackage.Module) {
- issues = append(issues, "additional_package.module must be a prefix of additional_package.package")
+ addIssue("additional_package.module must be a prefix of additional_package.package")
}
if additionalPackage.Package == "" {
additionalImportPath = additionalPackage.Module
@@ -192,42 +194,51 @@
additionalImportPath = additionalPackage.Package
}
if err := module.CheckImportPath(additionalImportPath); err != nil {
- issues = append(issues, err.Error())
+ addIssue(err.Error())
}
if !vuln.Stdlib {
if err := checkModVersions(additionalPackage.Module, additionalPackage.Versions); err != nil {
- issues = append(issues, err.Error())
+ addIssue(err.Error())
}
}
}
if vuln.Description == "" {
- issues = append(issues, "missing description")
+ addIssue("missing description")
}
if vuln.Published.IsZero() {
- issues = append(issues, "missing published")
+ addIssue("missing published")
}
if vuln.LastModified != nil && vuln.LastModified.Before(vuln.Published) {
- issues = append(issues, "last_modified is before published")
+ addIssue("last_modified is before published")
+ }
+
+ if vuln.CVE != "" && len(vuln.CVEs) > 0 {
+ addIssue("use only one of CVE and CVEs")
}
if vuln.CVE != "" && vuln.CVEMetadata != nil && vuln.CVEMetadata.ID != "" {
// TODO: may just want to use one of these? :shrug:
- issues = append(issues, "only one of cve and cve_metadata.id should be present")
+ addIssue("only one of cve and cve_metadata.id should be present")
}
if vuln.CVE != "" && !cveRegex.MatchString(vuln.CVE) {
issues = append(issues, "malformed cve identifier")
}
+ for _, cve := range vuln.CVEs {
+ if !cveRegex.MatchString(cve) {
+ addIssue("malformed cve identifier")
+ }
+ }
if vuln.CVEMetadata != nil {
if vuln.CVEMetadata.ID == "" {
- issues = append(issues, "cve_metadata.id is required")
+ addIssue("cve_metadata.id is required")
}
if !cveRegex.MatchString(vuln.CVEMetadata.ID) {
- issues = append(issues, "malformed cve_metadata.id identifier")
+ addIssue("malformed cve_metadata.id identifier")
}
}
diff --git a/internal/report/report.go b/internal/report/report.go
index f0a54d2..c33da0a 100644
--- a/internal/report/report.go
+++ b/internal/report/report.go
@@ -59,7 +59,10 @@
// CVE is the CVE ID for an existing CVE. If we are assigning a CVE ID
// ourselves, use CVEMetdata.ID instead.
- CVE string `yaml:",omitempty"`
+ CVE string `yaml:",omitempty"`
+ // CVE are CVE IDs for existing CVEs, if there is more than one.
+ // Use either CVE or CVEs, but not both.
+ CVEs []string `yaml:",omitempty"`
Credit string `yaml:",omitempty"`
Symbols []string `yaml:",omitempty"`
OS []string `yaml:",omitempty"`
diff --git a/osv/json.go b/osv/json.go
index 59f1a63..63e107f 100644
--- a/osv/json.go
+++ b/osv/json.go
@@ -243,6 +243,8 @@
if r.CVE != "" {
entry.Aliases = []string{r.CVE}
+ } else {
+ entry.Aliases = r.CVEs
}
var modules []string
diff --git a/vlint/vlint.go b/vlint/vlint.go
index 03ee6c0..d17ce48 100644
--- a/vlint/vlint.go
+++ b/vlint/vlint.go
@@ -15,8 +15,8 @@
)
// LintReport is used to lint the x/vulndb/reports/ directory. It is run by
-// TestLintReports and cmd/gendb to ensure that there are no errors in the YAML
-// reports.
+// TestLintReports (in the vulndb repo) to ensure that there are no errors in
+// the YAML reports.
func LintReport(filename string) (_ []string, err error) {
defer derrors.Wrap(&err, "Lint(%q)", filename)
b, err := ioutil.ReadFile(filename)
