From 620fc19dd23a5f9c0395cd84b820799099601de1 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Wed, 18 Oct 2023 12:57:37 +0600 Subject: [PATCH 1/4] fix: add missed primaryURL and source severity for CycloneDX --- .../testdata/pom-cyclonedx.json.golden | 6 +++++ pkg/sbom/cyclonedx/core/cyclonedx.go | 27 ++++++++++++++++--- pkg/sbom/cyclonedx/marshal_test.go | 21 ++++++++------- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/integration/testdata/pom-cyclonedx.json.golden b/integration/testdata/pom-cyclonedx.json.golden index 8e1c482e0580..3e87c28beedd 100644 --- a/integration/testdata/pom-cyclonedx.json.golden +++ b/integration/testdata/pom-cyclonedx.json.golden @@ -149,6 +149,9 @@ "description": "FasterXML jackson-databind 2.x before 2.9.10.4 mishandles the interaction between serialization gadgets and typing, related to br.com.anteros.dbcp.AnterosDBCPConfig (aka anteros-core).", "recommendation": "Upgrade com.fasterxml.jackson.core:jackson-databind to version 2.9.10.4", "advisories": [ + { + "url": "https://avd.aquasec.com/nvd/cve-2020-9548" + }, { "url": "https://access.redhat.com/security/cve/CVE-2020-9548" }, @@ -268,6 +271,9 @@ "description": "A flaw was found in jackson-databind before 2.9.10.7. FasterXML mishandles the interaction between serialization gadgets and typing. The highest threat from this vulnerability is to data confidentiality and integrity as well as system availability.", "recommendation": "Upgrade com.fasterxml.jackson.core:jackson-databind to version 2.9.10.7", "advisories": [ + { + "url": "https://avd.aquasec.com/nvd/cve-2021-20190" + }, { "url": "https://access.redhat.com/security/cve/CVE-2021-20190" }, diff --git a/pkg/sbom/cyclonedx/core/cyclonedx.go b/pkg/sbom/cyclonedx/core/cyclonedx.go index 9a2a7a14466f..9d40ac6726c4 100644 --- a/pkg/sbom/cyclonedx/core/cyclonedx.go +++ b/pkg/sbom/cyclonedx/core/cyclonedx.go @@ -155,7 +155,7 @@ func (c *CycloneDX) marshalVulnerability(bomRef string, vuln types.DetectedVulne Ratings: cdxRatings(vuln), CWEs: cwes(vuln.CweIDs), Description: vuln.Description, - Advisories: cdxAdvisories(vuln.References), + Advisories: cdxAdvisories(vuln.PrimaryURL, vuln.References), } if vuln.FixedVersion != "" { v.Recommendation = fmt.Sprintf("Upgrade %s to version %s", vuln.PkgName, vuln.FixedVersion) @@ -340,15 +340,22 @@ func UnmarshalProperties(properties *[]cdx.Property) map[string]string { return props } -func cdxAdvisories(refs []string) *[]cdx.Advisory { +func cdxAdvisories(primaryURL string, refs []string) *[]cdx.Advisory { // cyclonedx converts link to empty `[]cdx.Advisory` to `null` // `bom-1.5.schema.json` doesn't support this - `Invalid type. Expected: array, given: null` // we need to explicitly set `nil` for empty `refs` slice - if len(refs) == 0 { + if len(refs) == 0 && primaryURL == "" { return nil } var advs []cdx.Advisory + // CycloneDX has no equivalent `primaryURL` field + // Add `primaryURL` to the list of advisories to preserve identity with `json` format + if primaryURL != "" && !slices.Contains(refs, primaryURL) { + advs = append(advs, cdx.Advisory{ + URL: primaryURL, + }) + } for _, ref := range refs { advs = append(advs, cdx.Advisory{ URL: ref, @@ -377,6 +384,20 @@ func cwes(cweIDs []string) *[]int { func cdxRatings(vuln types.DetectedVulnerability) *[]cdx.VulnerabilityRating { rates := make([]cdx.VulnerabilityRating, 0) // nolint:gocritic // To export an empty array in JSON + if vuln.VulnerabilityID == "CVE-2022-0563" { + fmt.Println() + } + // + if _, ok := vuln.VendorSeverity[vuln.SeveritySource]; !ok { + severity, _ := dtypes.NewSeverity(vuln.Severity) + rate := cdx.VulnerabilityRating{ + Source: &cdx.Source{ + Name: string(vuln.SeveritySource), + }, + Severity: toCDXSeverity(severity), + } + rates = append(rates, rate) + } for sourceID, severity := range vuln.VendorSeverity { // When the vendor also provides CVSS score/vector if cvss, ok := vuln.CVSS[sourceID]; ok { diff --git a/pkg/sbom/cyclonedx/marshal_test.go b/pkg/sbom/cyclonedx/marshal_test.go index de41d837f453..2376c4f29b02 100644 --- a/pkg/sbom/cyclonedx/marshal_test.go +++ b/pkg/sbom/cyclonedx/marshal_test.go @@ -91,8 +91,7 @@ func TestMarshaler_Marshal(t *testing.T) { Description: "In GNU Binutils 2.31.1, there is a use-after-free in the error function in elfcomm.c when called from the process_archive function in readelf.c via a crafted ELF file.", Severity: dtypes.SeverityMedium.String(), VendorSeverity: dtypes.VendorSeverity{ - vulnerability.NVD: dtypes.SeverityMedium, - vulnerability.RedHatOVAL: dtypes.SeverityMedium, + vulnerability.NVD: dtypes.SeverityMedium, }, CweIDs: []string{"CWE-416"}, CVSS: dtypes.VendorCVSS{ @@ -102,10 +101,6 @@ func TestMarshaler_Marshal(t *testing.T) { V2Score: 4.3, V3Score: 5.5, }, - vulnerability.RedHatOVAL: dtypes.CVSS{ - V3Vector: "CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L", - V3Score: 5.3, - }, }, PublishedDate: lo.ToPtr(time.Date(2018, 12, 31, 19, 29, 0, 0, time.UTC)), LastModifiedDate: lo.ToPtr(time.Date(2019, 10, 31, 1, 15, 0, 0, time.UTC)), @@ -510,10 +505,7 @@ func TestMarshaler_Marshal(t *testing.T) { Name: string(vulnerability.RedHatOVAL), URL: "", }, - Score: lo.ToPtr(5.3), Severity: cdx.SeverityMedium, - Method: cdx.ScoringMethodCVSSv3, - Vector: "CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L", }, }, CWEs: &[]int{ @@ -522,6 +514,11 @@ func TestMarshaler_Marshal(t *testing.T) { Description: "In GNU Binutils 2.31.1, there is a use-after-free in the error function in elfcomm.c when called from the process_archive function in readelf.c via a crafted ELF file.", Published: "2018-12-31T19:29:00+00:00", Updated: "2019-10-31T01:15:00+00:00", + Advisories: &[]cdx.Advisory{ + { + URL: "https://avd.aquasec.com/nvd/cve-2018-20623", + }, + }, Affects: &[]cdx.Affects{ { Ref: "pkg:rpm/centos/binutils@2.30-93.el8?arch=aarch64&distro=centos-8.3.2011", @@ -991,6 +988,9 @@ func TestMarshaler_Marshal(t *testing.T) { }, Description: "Action Pack is a framework for handling and responding to web requests. Under certain circumstances response bodies will not be closed. In the event a response is *not* notified of a `close`, `ActionDispatch::Executor` will not know to reset thread local state for the next request. This can lead to data being leaked to subsequent requests.This has been fixed in Rails 7.0.2.1, 6.1.4.5, 6.0.4.5, and 5.2.6.1. Upgrading is highly recommended, but to work around this problem a middleware described in GHSA-wh98-p28r-vrc9 can be used.", Advisories: &[]cdx.Advisory{ + { + URL: "https://avd.aquasec.com/nvd/cve-2022-23633", + }, { URL: "http://www.openwall.com/lists/oss-security/2022/02/11/5", }, @@ -1384,6 +1384,9 @@ func TestMarshaler_Marshal(t *testing.T) { CWEs: lo.ToPtr([]int{94}), Description: "The DBCPConnectionPool and HikariCPConnectionPool Controller Services in Apache NiFi 0.0.2 through 1.21.0...", Advisories: &[]cdx.Advisory{ + { + URL: "https://avd.aquasec.com/nvd/cve-2023-34468", + }, { URL: "http://www.openwall.com/lists/oss-security/2023/06/12/3", }, From 581a750618bdec197d03f2d22d2cfb9d660ba072 Mon Sep 17 00:00:00 2001 From: DmitriyLewen Date: Wed, 18 Oct 2023 14:28:28 +0600 Subject: [PATCH 2/4] refactor: keep source severity in VendorSeverity not only for cyclonedx --- pkg/sbom/cyclonedx/core/cyclonedx.go | 14 -------------- pkg/sbom/cyclonedx/marshal_test.go | 10 +++++++++- pkg/vulnerability/vulnerability.go | 10 ++++++++++ pkg/vulnerability/vulnerability_test.go | 9 ++++++--- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/pkg/sbom/cyclonedx/core/cyclonedx.go b/pkg/sbom/cyclonedx/core/cyclonedx.go index 9d40ac6726c4..72a3c2c5f1c1 100644 --- a/pkg/sbom/cyclonedx/core/cyclonedx.go +++ b/pkg/sbom/cyclonedx/core/cyclonedx.go @@ -384,20 +384,6 @@ func cwes(cweIDs []string) *[]int { func cdxRatings(vuln types.DetectedVulnerability) *[]cdx.VulnerabilityRating { rates := make([]cdx.VulnerabilityRating, 0) // nolint:gocritic // To export an empty array in JSON - if vuln.VulnerabilityID == "CVE-2022-0563" { - fmt.Println() - } - // - if _, ok := vuln.VendorSeverity[vuln.SeveritySource]; !ok { - severity, _ := dtypes.NewSeverity(vuln.Severity) - rate := cdx.VulnerabilityRating{ - Source: &cdx.Source{ - Name: string(vuln.SeveritySource), - }, - Severity: toCDXSeverity(severity), - } - rates = append(rates, rate) - } for sourceID, severity := range vuln.VendorSeverity { // When the vendor also provides CVSS score/vector if cvss, ok := vuln.CVSS[sourceID]; ok { diff --git a/pkg/sbom/cyclonedx/marshal_test.go b/pkg/sbom/cyclonedx/marshal_test.go index 2376c4f29b02..ddc68131ea0c 100644 --- a/pkg/sbom/cyclonedx/marshal_test.go +++ b/pkg/sbom/cyclonedx/marshal_test.go @@ -91,7 +91,8 @@ func TestMarshaler_Marshal(t *testing.T) { Description: "In GNU Binutils 2.31.1, there is a use-after-free in the error function in elfcomm.c when called from the process_archive function in readelf.c via a crafted ELF file.", Severity: dtypes.SeverityMedium.String(), VendorSeverity: dtypes.VendorSeverity{ - vulnerability.NVD: dtypes.SeverityMedium, + vulnerability.NVD: dtypes.SeverityMedium, + vulnerability.RedHatOVAL: dtypes.SeverityMedium, }, CweIDs: []string{"CWE-416"}, CVSS: dtypes.VendorCVSS{ @@ -101,6 +102,10 @@ func TestMarshaler_Marshal(t *testing.T) { V2Score: 4.3, V3Score: 5.5, }, + vulnerability.RedHatOVAL: dtypes.CVSS{ + V3Vector: "CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L", + V3Score: 5.3, + }, }, PublishedDate: lo.ToPtr(time.Date(2018, 12, 31, 19, 29, 0, 0, time.UTC)), LastModifiedDate: lo.ToPtr(time.Date(2019, 10, 31, 1, 15, 0, 0, time.UTC)), @@ -505,7 +510,10 @@ func TestMarshaler_Marshal(t *testing.T) { Name: string(vulnerability.RedHatOVAL), URL: "", }, + Score: lo.ToPtr(5.3), Severity: cdx.SeverityMedium, + Method: cdx.ScoringMethodCVSSv3, + Vector: "CVSS:3.0/AV:L/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L", }, }, CWEs: &[]int{ diff --git a/pkg/vulnerability/vulnerability.go b/pkg/vulnerability/vulnerability.go index 2b29e632e0f6..f61d35a9f5c5 100644 --- a/pkg/vulnerability/vulnerability.go +++ b/pkg/vulnerability/vulnerability.go @@ -91,6 +91,16 @@ func (c Client) FillInfo(vulns []types.DetectedVulnerability) { if vulns[i].SeveritySource != "" { severity = vulns[i].Severity severitySource = vulns[i].SeveritySource + // Package-specific severity can be omitted (e.g. for Debian) + s, _ := dbTypes.NewSeverity(severity) // skip error handling because `SeverityUnknown` will be returned in case of error + if vuln.VendorSeverity != nil { + vuln.VendorSeverity[severitySource] = s + } else { + vuln.VendorSeverity = map[dbTypes.SourceID]dbTypes.Severity{ + severitySource: s, + } + } + } // Add the vulnerability detail diff --git a/pkg/vulnerability/vulnerability_test.go b/pkg/vulnerability/vulnerability_test.go index 8caf314ae7fa..11dac691503e 100644 --- a/pkg/vulnerability/vulnerability_test.go +++ b/pkg/vulnerability/vulnerability_test.go @@ -218,9 +218,12 @@ func TestClient_FillInfo(t *testing.T) { Status: dbTypes.StatusAffected, SeveritySource: vulnerability.Debian, Vulnerability: dbTypes.Vulnerability{ - Title: "dos", - Description: "dos vulnerability", - Severity: dbTypes.SeverityLow.String(), + Title: "dos", + Description: "dos vulnerability", + Severity: dbTypes.SeverityLow.String(), + VendorSeverity: map[dbTypes.SourceID]dbTypes.Severity{ + vulnerability.Debian: dbTypes.SeverityLow, + }, References: []string{"http://example.com"}, LastModifiedDate: utils.MustTimeParse("2020-01-01T01:01:00Z"), PublishedDate: utils.MustTimeParse("2001-01-01T01:01:00Z"), From 79bf8a868d45717785d0d51d4082d9ee72b8bb09 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Thu, 19 Oct 2023 12:59:08 +0900 Subject: [PATCH 3/4] refactor: make primary URL agnostic Signed-off-by: knqyf263 --- pkg/sbom/cyclonedx/core/cyclonedx.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/pkg/sbom/cyclonedx/core/cyclonedx.go b/pkg/sbom/cyclonedx/core/cyclonedx.go index 72a3c2c5f1c1..dad245b5239c 100644 --- a/pkg/sbom/cyclonedx/core/cyclonedx.go +++ b/pkg/sbom/cyclonedx/core/cyclonedx.go @@ -155,7 +155,7 @@ func (c *CycloneDX) marshalVulnerability(bomRef string, vuln types.DetectedVulne Ratings: cdxRatings(vuln), CWEs: cwes(vuln.CweIDs), Description: vuln.Description, - Advisories: cdxAdvisories(vuln.PrimaryURL, vuln.References), + Advisories: cdxAdvisories(append([]string{vuln.PrimaryURL}, vuln.References...)), } if vuln.FixedVersion != "" { v.Recommendation = fmt.Sprintf("Upgrade %s to version %s", vuln.PkgName, vuln.FixedVersion) @@ -340,27 +340,19 @@ func UnmarshalProperties(properties *[]cdx.Property) map[string]string { return props } -func cdxAdvisories(primaryURL string, refs []string) *[]cdx.Advisory { +func cdxAdvisories(refs []string) *[]cdx.Advisory { + refs = lo.Uniq(refs) + advs := lo.FilterMap(refs, func(ref string, _ int) (cdx.Advisory, bool) { + return cdx.Advisory{URL: ref}, ref != "" + }) + // cyclonedx converts link to empty `[]cdx.Advisory` to `null` // `bom-1.5.schema.json` doesn't support this - `Invalid type. Expected: array, given: null` // we need to explicitly set `nil` for empty `refs` slice - if len(refs) == 0 && primaryURL == "" { + if len(advs) == 0 { return nil } - var advs []cdx.Advisory - // CycloneDX has no equivalent `primaryURL` field - // Add `primaryURL` to the list of advisories to preserve identity with `json` format - if primaryURL != "" && !slices.Contains(refs, primaryURL) { - advs = append(advs, cdx.Advisory{ - URL: primaryURL, - }) - } - for _, ref := range refs { - advs = append(advs, cdx.Advisory{ - URL: ref, - }) - } return &advs } From b146a9758ded440624f361df657727d5dcfd4df7 Mon Sep 17 00:00:00 2001 From: knqyf263 Date: Thu, 19 Oct 2023 13:03:57 +0900 Subject: [PATCH 4/4] refactor: remove else statement Signed-off-by: knqyf263 --- pkg/vulnerability/vulnerability.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/vulnerability/vulnerability.go b/pkg/vulnerability/vulnerability.go index f61d35a9f5c5..504d9293e873 100644 --- a/pkg/vulnerability/vulnerability.go +++ b/pkg/vulnerability/vulnerability.go @@ -91,16 +91,13 @@ func (c Client) FillInfo(vulns []types.DetectedVulnerability) { if vulns[i].SeveritySource != "" { severity = vulns[i].Severity severitySource = vulns[i].SeveritySource - // Package-specific severity can be omitted (e.g. for Debian) - s, _ := dbTypes.NewSeverity(severity) // skip error handling because `SeverityUnknown` will be returned in case of error - if vuln.VendorSeverity != nil { - vuln.VendorSeverity[severitySource] = s - } else { - vuln.VendorSeverity = map[dbTypes.SourceID]dbTypes.Severity{ - severitySource: s, - } - } + // Store package-specific severity in vendor severities + if vuln.VendorSeverity == nil { + vuln.VendorSeverity = make(dbTypes.VendorSeverity) + } + s, _ := dbTypes.NewSeverity(severity) // skip error handling because `SeverityUnknown` will be returned in case of error + vuln.VendorSeverity[severitySource] = s } // Add the vulnerability detail