feat(report): add CVSS vectors in sarif report

[axidex]

Description

Adding CVSS vector information to Rule.Properties. This was previously added to the JSON output in PR #484.

Output

Before

{
  "version": "2.1.0",
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json",
  "runs": [
    {
      "tool": {
        "driver": {
          "fullName": "Trivy Vulnerability Scanner",
          "informationUri": "https://github.com/aquasecurity/trivy",
          "name": "Trivy",
          "rules": [
            {
              "id": "CVE-2019-1549",
              "name": "OsPackageVulnerability",
              "shortDescription": {
                "text": "openssl: information disclosure in fork()"
              },
              "fullDescription": {
                "text": "OpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c)."
              },
              "defaultConfiguration": {
                "level": "warning"
              },
              "helpUri": "https://avd.aquasec.com/nvd/cve-2019-1549",
              "help": {
                "text": "Vulnerability CVE-2019-1549\nSeverity: MEDIUM\nPackage: libssl1.1\nFixed Version: 1.1.1d-r0\nLink: [CVE-2019-1549](https://avd.aquasec.com/nvd/cve-2019-1549)\nOpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c).",
                "markdown": "**Vulnerability CVE-2019-1549**\n| Severity | Package | Fixed Version | Link |\n| --- | --- | --- | --- |\n|MEDIUM|libssl1.1|1.1.1d-r0|[CVE-2019-1549](https://avd.aquasec.com/nvd/cve-2019-1549)|\n\nOpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c)."
              },
              "properties": {
                "precision": "very-high",
                "security-severity": "5.3",
                "tags": [
                  "vulnerability",
                  "security",
                  "MEDIUM"
                ]
              }
            }
          ],
          "version": "dev"
        }
      },
      "results": [...],
      "columnKind": "utf16CodeUnits",
      "originalUriBaseIds": {
        "ROOTPATH": {
          "uri": "file:///"
        }
      },
      "properties": {
        "imageID": "sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4",
        "imageName": "testdata/fixtures/images/alpine-310.tar.gz",
        "repoDigests": null,
        "repoTags": null
      }
    }
  ]
}

After

{
  "version": "2.1.0",
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/sarif-2.1/schema/sarif-schema-2.1.0.json",
  "runs": [
    {
      "tool": {
        "driver": {
          "fullName": "Trivy Vulnerability Scanner",
          "informationUri": "https://github.com/aquasecurity/trivy",
          "name": "Trivy",
          "rules": [
            {
              "id": "CVE-2019-1549",
              "name": "OsPackageVulnerability",
              "shortDescription": {
                "text": "openssl: information disclosure in fork()"
              },
              "fullDescription": {
                "text": "OpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c)."
              },
              "defaultConfiguration": {
                "level": "warning"
              },
              "helpUri": "https://avd.aquasec.com/nvd/cve-2019-1549",
              "help": {
                "text": "Vulnerability CVE-2019-1549\nSeverity: MEDIUM\nPackage: libssl1.1\nFixed Version: 1.1.1d-r0\nLink: [CVE-2019-1549](https://avd.aquasec.com/nvd/cve-2019-1549)\nOpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c).",
                "markdown": "**Vulnerability CVE-2019-1549**\n| Severity | Package | Fixed Version | Link |\n| --- | --- | --- | --- |\n|MEDIUM|libssl1.1|1.1.1d-r0|[CVE-2019-1549](https://avd.aquasec.com/nvd/cve-2019-1549)|\n\nOpenSSL 1.1.1 introduced a rewritten random number generator (RNG). This was intended to include protection in the event of a fork() system call in order to ensure that the parent and child processes did not share the same RNG state. However this protection was not being used in the default case. A partial mitigation for this issue is that the output from a high precision timer is mixed into the RNG state so the likelihood of a parent and child process sharing state is significantly reduced. If an application already calls OPENSSL_init_crypto() explicitly using OPENSSL_INIT_ATFORK then this problem does not occur at all. Fixed in OpenSSL 1.1.1d (Affected 1.1.1-1.1.1c)."
              },
              "properties": {
                "cvss": {
                  "cvssv2_vector": "AV:N/AC:L/Au:N/C:P/I:N/A:N",
                  "cvssv3_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N",
                  "cvssv2_score": 5,
                  "cvssv3_baseScore": 5.3
                },
                "precision": "very-high",
                "security-severity": "5.3",
                "tags": [
                  "vulnerability",
                  "security",
                  "MEDIUM"
                ]
              }
            }
          ],
          "version": "dev"
        }
      },
      "results": [...],
      "columnKind": "utf16CodeUnits",
      "originalUriBaseIds": {
        "ROOTPATH": {
          "uri": "file:///"
        }
      },
      "properties": {
        "imageID": "sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4",
        "imageName": "testdata/fixtures/images/alpine-310.tar.gz",
        "repoDigests": null,
        "repoTags": null
      }
    }
  ]
}

Properties in rules before

"properties": {
  "precision": "very-high",
  "security-severity": "5.3",
  "tags": [
    "vulnerability",
    "security",
    "MEDIUM"
  ]
}

Properties in rules after

"properties": {
  "cvss": {
    "cvssv2_vector": "AV:N/AC:L/Au:N/C:P/I:N/A:N",
    "cvssv3_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N",
    "cvssv2_score": 5,
    "cvssv3_baseScore": 5.3
  },
  "precision": "very-high",
  "security-severity": "5.3",
  "tags": [
    "vulnerability",
    "security",
    "MEDIUM"
  ]
}

Related PRs

• vulnerability: Add CVSS Vectors to JSON output. #484

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[CLAassistant]

All committers have signed the CLA.

[DmitriyLewen]

nit: It makes no sense to add the value nil here.
Same for the other lines below.

[axidex]

tests aren't passing without nil failing here

--- Expected
+++ Actual
@@ -73,3 +73,4 @@
        MessageStrings: (*sarif.MessageStrings)(<nil>),
-       Properties: (sarif.Properties) (len=3) {
+       Properties: (sarif.Properties) (len=4) {
+        (string) (len=11) "cvss-vector": (interface {}) <nil>,

[axidex]

Fix it with this:
https://github.com/axidex/trivy/blob/527433bea25a10555ca8e81b03fa0606821a54d6/pkg/report/sarif.go#L473-L475

[DmitriyLewen]

Do you have documentation about this field or official examples?
We need to make sure we are using the correct names/structure for this property.

[orchestr7]

It is called cvss in json output, @axidex you can follow that naming, I guess

[axidex]

renamed field from cvss-vector to cvss
JSON ref

[DmitriyLewen]

This is our (Trivy) field name.
But I told about sarif schema.
Does sarif use similar fields?

[axidex]

This is our (Trivy) field name. But I told about sarif schema. Does sarif use similar fields?

I didn't see anything like that in semgrep, codeql or any other tools

[axidex]

Created a structure for this:

type CVSSData struct {
	CVSSV2Vector string  `json:"cvssv2_vector,omitempty"`
	CVSSV2Score  float64 `json:"cvssv2_score,omitempty"`

	CVSSV3Vector string  `json:"cvssv3_vector,omitempty"`
	CVSSV3Score  float64 `json:"cvssv3_score,omitempty"`

	CVSSV40Vector string  `json:"cvssv40_vector,omitempty"`
	CVSSV40Score  float64 `json:"cvssv40_score,omitempty"`
}

[DmitriyLewen]

No need to create struct for this.
cvssv2_score is property. (same for other fields)
So should be (example):

              "properties": {
                "cvssv2_vector": "AV:N/AC:L/Au:N/C:P/I:N/A:N",
                "cvssv2_score": 5,
                "cvssv3_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N",
                  "cvssv3_score": 5.3
                "precision": "very-high",

[axidex]

No need to create struct for this. cvssv2_score is property. (same for other fields) So should be (example):

              "properties": {
                "cvssv2_vector": "AV:N/AC:L/Au:N/C:P/I:N/A:N",
                "cvssv2_score": 5,
                "cvssv3_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N",
                  "cvssv3_score": 5.3
                "precision": "very-high",

Changed this in the new version

[DmitriyLewen]

LGTM.
I created small refactoring, can you take a look:

diff --git a/pkg/report/sarif.go b/pkg/report/sarif.go
index 2e2268450..ee7f2b87b 100644
--- a/pkg/report/sarif.go
+++ b/pkg/report/sarif.go
@@ -43,17 +43,6 @@ var (
 	pathRegex = regexp.MustCompile(`(?P<path>.+?)(?:\s*\((?:.*?)\).*?)?$`)
 )
 
-type CVSSData struct {
-	CVSSV2Vector string
-	CVSSV2Score  float64
-
-	CVSSV3Vector string
-	CVSSV3Score  float64
-
-	CVSSV40Vector string
-	CVSSV40Score  float64
-}
-
 // SarifWriter implements result Writer
 type SarifWriter struct {
 	Output        io.Writer
@@ -78,7 +67,7 @@ type sarifData struct {
 	locationMessage  string
 	message          string
 	cvssScore        string
-	cvssData         CVSSData
+	cvssData         map[string]any
 	locations        []location
 }
 
@@ -100,7 +89,7 @@ func (sw *SarifWriter) addSarifRule(data *sarifData) {
 		WithDefaultConfiguration(&sarif.ReportingConfiguration{
 			Level: toSarifErrorLevel(data.severity),
 		}).
-		WithProperties(MakeProperties(data.title, data.severity, data.cvssScore, data.cvssData))
+		WithProperties(toProperties(data.title, data.severity, data.cvssScore, data.cvssData))
 	if data.url != nil && data.url.String() != "" {
 		r.WithHelpURI(data.url.String())
 	}
@@ -162,12 +151,13 @@ func (sw *SarifWriter) Write(_ context.Context, report types.Report) error {
 			if vuln.PkgPath != "" {
 				path = ToPathUri(vuln.PkgPath, res.Class)
 			}
+			cvssData, cvssScore := toCVSSData(vuln)
 			sw.addSarifResult(&sarifData{
 				title:            "vulnerability",
 				vulnerabilityId:  vuln.VulnerabilityID,
 				severity:         vuln.Severity,
-				cvssScore:        getCVSSScore(vuln),
-				cvssData:         getCVSSData(vuln),
+				cvssScore:        cvssScore,
+				cvssData:         cvssData,
 				url:              toUri(vuln.PrimaryURL),
 				resourceClass:    res.Class,
 				artifactLocation: toUri(path),
@@ -419,29 +409,25 @@ func (sw *SarifWriter) getLocations(name, version, path string, pkgs []ftypes.Pa
 	return locs
 }
 
-func getCVSSScore(vuln types.DetectedVulnerability) string {
-	// Take the vendor score
-	if cvss, ok := vuln.CVSS[vuln.SeveritySource]; ok {
-		return fmt.Sprintf("%.1f", cvss.V3Score)
-	}
-
-	// Converts severity to score
-	return severityToScore(vuln.Severity)
-}
-
-func getCVSSData(vuln types.DetectedVulnerability) CVSSData {
+// toCVSSData extracts CVSS data from the vulnerability and returns it along with the score.
+// If CVSS V3 Score is not available, it returns an empty CVSSData struct and a score based on severity.
+func toCVSSData(vuln types.DetectedVulnerability) (map[string]any, string) {
+	score := severityToScore(vuln.Severity)
+	var data = make(map[string]any)
 	if cvss, ok := vuln.CVSS[vuln.SeveritySource]; ok {
-		return CVSSData{
-			CVSSV2Score:   cvss.V2Score,
-			CVSSV2Vector:  cvss.V2Vector,
-			CVSSV3Score:   cvss.V3Score,
-			CVSSV3Vector:  cvss.V3Vector,
-			CVSSV40Score:  cvss.V40Score,
-			CVSSV40Vector: cvss.V40Vector,
+		data["cvssv2_vector"] = cvss.V2Vector
+		data["cvssv2_score"] = cvss.V2Score
+		data["cvssv3_vector"] = cvss.V3Vector
+		data["cvssv3_score"] = cvss.V3Score
+		data["cvssv40_vector"] = cvss.V40Vector
+		data["cvssv40_score"] = cvss.V40Score
+
+		if cvss.V3Score != 0 {
+			score = fmt.Sprintf("%.1f", cvss.V3Score)
 		}
 	}
 
-	return CVSSData{}
+	return data, score
 }
 
 func severityToScore(severity string) string {
@@ -459,7 +445,7 @@ func severityToScore(severity string) string {
 	}
 }
 
-func MakeProperties(title, severity, cvssScore string, cvssData CVSSData) sarif.Properties {
+func toProperties(title, severity, cvssScore string, cvssData map[string]any) sarif.Properties {
 	properties := sarif.Properties{
 		"tags": []string{
 			title,
@@ -470,28 +456,18 @@ func MakeProperties(title, severity, cvssScore string, cvssData CVSSData) sarif.
 		"security-severity": cvssScore,
 	}
 
-	// Add CVSS v2
-	if cvssData.CVSSV2Vector != "" {
-		properties["cvssv2_vector"] = cvssData.CVSSV2Vector
-	}
-	if cvssData.CVSSV2Score != 0 {
-		properties["cvssv2_score"] = cvssData.CVSSV2Score
-	}
-
-	// Add CVSS v3
-	if cvssData.CVSSV3Vector != "" {
-		properties["cvssv3_vector"] = cvssData.CVSSV3Vector
-	}
-	if cvssData.CVSSV3Score != 0 {
-		properties["cvssv3_score"] = cvssData.CVSSV3Score
-	}
-
-	// Add CVSS v4
-	if cvssData.CVSSV40Vector != "" {
-		properties["cvssv40_vector"] = cvssData.CVSSV40Vector
-	}
-	if cvssData.CVSSV40Score != 0 {
-		properties["cvssv40_score"] = cvssData.CVSSV40Score
+	for key, value := range cvssData {
+		switch v := value.(type) {
+		case string:
+			if v == "" {
+				continue
+			}
+		case float64:
+			if v == 0 {
+				continue
+			}
+		}
+		properties[key] = value
 	}
 
 	return properties

[axidex]

Thank you for the refactoring! I have made changes based on your suggestions.

[DmitriyLewen]

@axidex Thanks for your work!

One quick question: where do you plan to use these files (with the new fields).
Why not use Trivy JSON format, for example?

[axidex]

@axidex Thanks for your work!

One quick question: where do you plan to use these files (with the new fields). Why not use Trivy JSON format, for example?

I used it in the orchestration of security testing tools. The unified entry point for completing scans is the parsing of SARIF reports. I have a unified structure for SARIF, but it is not compatible with JSON. This fields will be used to create combined report about whole system(based on SCA, SS, SAST, DAST/MDAST e.t.c.).

[DmitriyLewen]

LGTM.
Left 1 small comment.

@knqyf263, can you take a look too?
aws doesn't use all the fields you add, but the changes preserve their logic.

[DmitriyLewen]

I copied the keys incorrectly.
aws uses cvssv3_baseScore.
Can you update these fields?

[axidex]

Done.

[DmitriyLewen]

wdyt about cvssv40_score.
aws used another name for v3 (score -> baseScore)
perhaps make sense use "new" format for v40?
i mean cvssv40_baseScore

[axidex]

wdyt about cvssv40_score. aws used another name for v3 (score -> baseScore) perhaps make sense use "new" format for v40? i mean cvssv40_baseScore

If u mean to make changes like this:
cvssv40_score -> cvssv40_baseScore
and
cvssv3_baseScore -> cvssv3_score
I think it's good. But we should migrate this too:

if cvss.V3Score != 0 {
	score = fmt.Sprintf("%.1f", cvss.V3Score)
}

[DmitriyLewen]

My suggest:

cvssv2_vector
cvssv2_score
cvssv3_vector
cvssv3_baseScore
cvssv40_vector
cvssv40_baseScore

[axidex]

Changed from cvssv40_score to cvssv40_baseScore

[DmitriyLewen]

Thanks for your contribution!
LGTM

@knqyf263 can you quick a look?

[knqyf263]

Could you add a comment on why we chose cvssv3_baseScore with a mix of camel and snake cases? I guess you referenced https://docs.aws.amazon.com/codecatalyst/latest/userguide/test.sarif.html, but I would like to clarify it for future reference.

[axidex]

Could you add a comment on why we chose cvssv3_baseScore with a mix of camel and snake cases? I guess you referenced https://docs.aws.amazon.com/codecatalyst/latest/userguide/test.sarif.html, but I would like to clarify it for future reference.

Added comments in a073a27