From a0a99e95878e213a0fe00cb3efbe43136cc0583e Mon Sep 17 00:00:00 2001 From: felipecruz91 Date: Thu, 11 Jul 2024 12:30:19 +0200 Subject: [PATCH 1/3] fix(vex): allow VEX matching when no subcomponents Signed-off-by: felipecruz91 --- policy/data/vulnerabilities.go | 88 ++++++-- policy/data/vulnerabilities_test.go | 305 ++++++++++++++++++++++++++++ 2 files changed, 376 insertions(+), 17 deletions(-) diff --git a/policy/data/vulnerabilities.go b/policy/data/vulnerabilities.go index d9df81b..1d9fe64 100644 --- a/policy/data/vulnerabilities.go +++ b/policy/data/vulnerabilities.go @@ -3,6 +3,7 @@ package data import ( "context" + "github.com/openvex/go-vex/pkg/vex" govex "github.com/openvex/go-vex/pkg/vex" "github.com/atomist-skills/go-skill/policy/data/query" @@ -43,23 +44,7 @@ func (ds *DataSource) GetImageVulnerabilities(ctx context.Context, evalCtx goals normalization.DenormalizeSBOM(&vulnsResponse, purlMapping) for _, vulnsByPurl := range vulnsResponse.VulnerabilitiesByPackage { - affected := true - for _, v := range imageSbom.VexDocuments { - for _, stmt := range v.Statements { - purl, upstreamPurl := normalization.NormalizePURL(vulnsByPurl.Purl, nil) - for _, p := range stmt.Products { - if normalization.ContainsPurl(p.Subcomponents, purl) || normalization.ContainsPurl(p.Subcomponents, upstreamPurl) { - if stmt.Status == govex.StatusNotAffected || stmt.Status == govex.StatusFixed { - affected = false - } - } - } - } - } - - if affected { - vulns[vulnsByPurl.Purl] = vulnsByPurl.Vulnerabilities - } + vulns[vulnsByPurl.Purl] = applyVEX(vulnsByPurl, imageSbom.VexDocuments) } } else { var response jynx.ImagePackagesByDigestResponse @@ -80,3 +65,72 @@ func (ds *DataSource) GetImageVulnerabilities(ctx context.Context, evalCtx goals return &query.QueryResponse{}, packages, vulns, nil } + +// applyVEX returns the CVEs that remain relevant after cross-referencing them with VEX documents. +func applyVEX(vulnsByPurl types.VulnerabilitiesByPurl, vexDocs []vex.VEX) []types.Vulnerability { + filteredOutCVEs := []types.Vulnerability{} + + for _, cve := range vulnsByPurl.Vulnerabilities { + for _, v := range vexDocs { + for _, stmt := range v.Statements { + if cveMatch(cve.SourceId, stmt) { + if purlMatch(vulnsByPurl.Purl, stmt) { + if notAffectedOrFixed(stmt) { + filteredOutCVEs = append(filteredOutCVEs, cve) + } + } + } + } + } + } + + vexedCVEsMap := make(map[string]bool, len(filteredOutCVEs)) + for _, cve := range filteredOutCVEs { + vexedCVEsMap[cve.SourceId] = true + } + + // Filter out the VEXed CVEs + cves := make([]types.Vulnerability, 0, len(vulnsByPurl.Vulnerabilities)) + for _, cve := range vulnsByPurl.Vulnerabilities { + if !vexedCVEsMap[cve.SourceId] { + cves = append(cves, cve) + } + } + return cves +} + +// cveMatch checks whether a CVE is present in a VEX statement +func cveMatch(cveID string, stmt govex.Statement) bool { + return stmt.Vulnerability.ID == cveID || string(stmt.Vulnerability.Name) == cveID +} + +// purlMatch checks whether a purl is present in at least one of the following locations: +// - Component +// - Subcomponent(s) +// - Special case for org-scoped VEXed CVEs. +func purlMatch(purl string, stmt govex.Statement) bool { + purl, upstreamPurl := normalization.NormalizePURL(purl, nil) + + for _, p := range stmt.Products { + // Check if purl is defined as the top-level component + if purl == p.Component.ID { + return true + } + // Check if purl is defined as one of the subcomponents + if normalization.ContainsPurl(p.Subcomponents, purl) || normalization.ContainsPurl(p.Subcomponents, upstreamPurl) { + return true + } + // If none of the previous conditions matched, we add this special case to support org-scoped VEXed CVEs. + // The purpose of this is to align with how VEX works in the platform side. + if len(p.Subcomponents) == 0 { + return true + } + } + + return false +} + +// notAffectedOrFixed checks whether the statement status is not affected or fixed. +func notAffectedOrFixed(stmt govex.Statement) bool { + return stmt.Status == govex.StatusNotAffected || stmt.Status == govex.StatusFixed +} diff --git a/policy/data/vulnerabilities_test.go b/policy/data/vulnerabilities_test.go index 1185b20..85ae44d 100644 --- a/policy/data/vulnerabilities_test.go +++ b/policy/data/vulnerabilities_test.go @@ -4,11 +4,14 @@ import ( "context" "testing" + govex "github.com/openvex/go-vex/pkg/vex" + "github.com/atomist-skills/go-skill/internal/test_util" "github.com/atomist-skills/go-skill/policy/data/query" "github.com/atomist-skills/go-skill/policy/data/query/jynx" "github.com/atomist-skills/go-skill/policy/goals" "github.com/atomist-skills/go-skill/policy/types" + "github.com/openvex/go-vex/pkg/vex" "github.com/stretchr/testify/assert" ) @@ -338,6 +341,308 @@ func Test_GetImageVulnerabilities_WhenSbomHasNoArtifacts_AndNoVulnerabilities(t assert.Equal(t, expectedVulnerabilities, vulnerabilities) } +func Test_applyVEX(t *testing.T) { + const ( + openSSLPurl = "pkg:apk/alpine/openssl@3.0.12-r1?os_name=alpine&os_version=3.17" + alpineImgPurl = "pkg:docker/alpine@sha256:6e94b5cda2d6fd57d85abf81e81dabaea97a5885f919da676cc19d3551da4061" + awsPurl = "pkg:golang/github.com/aws/aws-sdk-go@1.44.288" + ) + + tests := []struct { + name string + vulnsByPurl types.VulnerabilitiesByPurl + vexDocs []vex.VEX + expectedCVEs []types.Vulnerability // CVEs after applying VEX + }{ + { + name: "CVE-2024-5535 is not filtered out when there aren't VEX documents", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535"), + }, + vexDocs: []vex.VEX{}, // empty on purpose + expectedCVEs: cves("CVE-2024-5535"), + }, + { + name: "CVE-2024-5535 is not filtered out when the VEX document has no statements", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{}, // empty on purpose + }, + }, + expectedCVEs: cves("CVE-2024-5535"), + }, + { + name: "CVE-2024-5535 is not filtered out when purl is not present in either the product id or subcomponents", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + ID: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: alpineImgPurl, + }, + Subcomponents: []vex.Subcomponent{ + { + Component: vex.Component{ + ID: awsPurl, + }, + }, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-5535"), + }, + { + name: "CVE-2024-5535 is filtered out when purl matches the product id", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535", "CVE-2024-5536"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + ID: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: openSSLPurl, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-5536"), + }, + { + name: "CVE-2024-5535 is filtered out when purl is present in subcomponents", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535", "CVE-2024-5536"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + ID: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: alpineImgPurl, + }, + Subcomponents: []vex.Subcomponent{ + { + Component: vex.Component{ + ID: openSSLPurl, + }, + }, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-5536"), + }, + { + name: "CVE-2024-5535 is filtered out when there are no subcomponents (even if there is a product id mismatch)", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535", "CVE-2024-5536"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + ID: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: alpineImgPurl, // notice product id mismatch with openSSLPurl + }, + Subcomponents: []vex.Subcomponent{}, // empty on purpose + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-5536"), + }, + { + name: "CVE-2024-0001 is not filtered out when its source id does not match the vulnerability id in the VEX statement", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-0001"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + ID: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: alpineImgPurl, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-0001"), + }, + { + name: "CVE-2024-0001 is not filtered out when its source id does not match the vulnerability name in the VEX statement", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-0001"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + Name: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: alpineImgPurl, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: cves("CVE-2024-0001"), + }, + { + name: "CVE-2024-5535 is filtered out when status is not_affected", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + Name: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: openSSLPurl, + }, + }, + }, + Status: govex.StatusNotAffected, + Justification: vex.VulnerableCodeNotInExecutePath, + }, + }, + }, + }, + expectedCVEs: []types.Vulnerability{}, + }, + { + name: "CVE-2024-5535 is filtered out when status is fixed", + vulnsByPurl: types.VulnerabilitiesByPurl{ + Purl: openSSLPurl, + Vulnerabilities: cves("CVE-2024-5535"), + }, + vexDocs: []vex.VEX{ + { + Statements: []vex.Statement{ + { + Vulnerability: vex.Vulnerability{ + Name: "CVE-2024-5535", + }, + Products: []vex.Product{ + { + Component: govex.Component{ + ID: openSSLPurl, + }, + }, + }, + Status: govex.StatusFixed, + }, + }, + }, + }, + expectedCVEs: []types.Vulnerability{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := applyVEX(tt.vulnsByPurl, tt.vexDocs) + if len(actual) != len(tt.expectedCVEs) { + t.Errorf("applyVEX() = %d, want %d", len(actual), len(tt.expectedCVEs)) + } + if len(actual) == len(tt.expectedCVEs) { + for i, v := range actual { + if tt.expectedCVEs[i].SourceId != v.SourceId { + t.Errorf("applyVEX() = %v, want %v", v.SourceId, tt.expectedCVEs[i].SourceId) + } + } + } + }) + } +} + +func cves(cveIDs ...string) []types.Vulnerability { + var cves = make([]types.Vulnerability, 0, len(cveIDs)) + for _, cve := range cveIDs { + cves = append(cves, types.Vulnerability{ + SourceId: cve, + }) + } + return cves +} + func Ptr[T any](v T) *T { return &v } From fd1d8fb6fb23789270cc834c7bb2c37ad5b62440 Mon Sep 17 00:00:00 2001 From: Felipe Cruz Martinez <15997951+felipecruz91@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:04:33 +0200 Subject: [PATCH 2/3] Apply suggestions from code review --- policy/data/vulnerabilities.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policy/data/vulnerabilities.go b/policy/data/vulnerabilities.go index 1d9fe64..a10aa14 100644 --- a/policy/data/vulnerabilities.go +++ b/policy/data/vulnerabilities.go @@ -120,7 +120,7 @@ func purlMatch(purl string, stmt govex.Statement) bool { if normalization.ContainsPurl(p.Subcomponents, purl) || normalization.ContainsPurl(p.Subcomponents, upstreamPurl) { return true } - // If none of the previous conditions matched, we add this special case to support org-scoped VEXed CVEs. + // If none of the previous conditions matched, we add this special case to support image-scoped exceptions. // The purpose of this is to align with how VEX works in the platform side. if len(p.Subcomponents) == 0 { return true From 2b8c07e8b03fb0d73a129dc44de3f231dd5f4349 Mon Sep 17 00:00:00 2001 From: Felipe Cruz Martinez <15997951+felipecruz91@users.noreply.github.com> Date: Fri, 12 Jul 2024 15:06:35 +0200 Subject: [PATCH 3/3] Apply suggestions from code review --- policy/data/vulnerabilities.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/policy/data/vulnerabilities.go b/policy/data/vulnerabilities.go index a10aa14..0ddd1ac 100644 --- a/policy/data/vulnerabilities.go +++ b/policy/data/vulnerabilities.go @@ -107,7 +107,7 @@ func cveMatch(cveID string, stmt govex.Statement) bool { // purlMatch checks whether a purl is present in at least one of the following locations: // - Component // - Subcomponent(s) -// - Special case for org-scoped VEXed CVEs. +// - Special case for image-scoped exceptions. func purlMatch(purl string, stmt govex.Statement) bool { purl, upstreamPurl := normalization.NormalizePURL(purl, nil)