refactor: add warning if severity not from vendor (or NVD or GH) is used #6726

DmitriyLewen · 2024-05-20T07:11:00Z

Description

See #6714 (comment)

Example:

Related issues

Close unexpected severity if nvd and source don't have severity for vulnerability #6714

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).

knqyf263 · 2024-05-28T07:44:24Z

pkg/vulnerability/vulnerability.go

@@ -130,6 +130,7 @@ func (c Client) getVendorSeverity(vulnID string, vuln *dbTypes.Vulnerability, so
 		return dbTypes.SeverityUnknown.String(), ""
 	}

+	log.Warn("Vendor and NVD don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", log.String("CVE", vulnID))


It's not necessarily CVE-ID.

Suggested change

log.Warn("Vendor and NVD don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", log.String("CVE", vulnID))

log.Warn("Vendor and NVD don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", log.String("vulnerability-id", vulnID))

Vendor is unclear. We may want to show the source (dbTypes.SourceID).

I thought about this, but there is 1 problem with this.
We fill severity filed in trivy-db:
https://github.com/aquasecurity/trivy-db/blob/b8fe1376ffcdc69fe454f0a8a481ab485e47aea5/pkg/vulnsrc/vulnerability/vulnerability.go#L92-L108

Therefore, we don't have info about vendor for this severity.

We can only check vendorSeverity and detect vendor with same severity.
But if 2 vendors use this severity, we may make mistake in our choice

I mean, we should say "Debian and NVD don't have severity" when scanning a Debian image and "Ubuntu and NVD don't have severity" when scanning an Ubuntu image. We need to consider what should be displayed with language-specific packages, though.

I understand you now.
We can use dataSource.Name (see :

It also works for language-specific packages

Co-authored-by: Teppei Fukuda <[email protected]>

…ndor-severity-missing-log

knqyf263 · 2024-06-03T05:51:16Z

pkg/vulnerability/vulnerability.go

@@ -130,6 +129,7 @@ func (c Client) getVendorSeverity(vulnID string, vuln *dbTypes.Vulnerability, so
 		return dbTypes.SeverityUnknown.String(), ""
 	}

+	log.Warn(fmt.Sprintf("%q and \"NVD\" don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", dataSource.Name), log.String("vulnerability-id", vulnID))


Don't we need sync.Once to prevent from showing the same messages many times?

I think we should show each vulnerability with non-vendor (non-nvd) severity.
It will be difficult (and possibly manual) for user to detect vulnerabilities with inappropriate severity levels if we simply say that some vulnerabilities use different severity levels.

As another way, we can save all these vulnerabilities and insert this list into one warning.
wdyt?

But if there are 100 vulnerabilities, we possibly show 100 warn messages, right?

right. but I hope we don't get 100 vulnerabilities

I don't think 100 vulns are so many. I saw environments with 500 vulns or even more than 1000 vulns.

Okay, let's use once. We can always update this - if users require it.

Update in b23527d

knqyf263 · 2024-06-10T03:08:49Z

pkg/vulnerability/vulnerability.go

+// Show warning if we use severity from another vendor
+// cf. https://github.com/aquasecurity/trivy/issues/6714
+var onceWarn = sync.OnceFunc(func() {
+	log.Warn("For some vulnerabilities, severities from other vendors were used because their data sources and NVD don't contain severities. You can view severity list from vendors in `json` format in `VendorSeverity` field.")


After reading the message, I feel like it's still unclear for users. We may want to update the document and add a link in the warning message, like "For details, read https://aquasecurity.github.io/trivy/latest/docs/scanner/vulnerability/#severity-selection." What do you think?

I have a couple of ideas that we can add. I will update docs and write to you.

ec412a3
wdyt?

Signed-off-by: knqyf263 <[email protected]>

refactor: add warning if severity not from vendor (or NVD or GH) is used

4e26d52

DmitriyLewen self-assigned this May 20, 2024

DmitriyLewen marked this pull request as ready for review May 20, 2024 07:20

DmitriyLewen requested a review from knqyf263 as a code owner May 20, 2024 07:20

knqyf263 reviewed May 28, 2024

View reviewed changes

DmitriyLewen and others added 3 commits May 28, 2024 13:53

refactor: use vulnerability-id

d5c7146

Co-authored-by: Teppei Fukuda <[email protected]>

Merge branch 'main' of github.com:DmitriyLewen/trivy into refactor/ve…

5769f97

…ndor-severity-missing-log

refactor: use dataSource.Name in warning

0d1c12f

knqyf263 reviewed Jun 3, 2024

View reviewed changes

refactor: use onceFunc

b23527d

knqyf263 reviewed Jun 10, 2024

View reviewed changes

DmitriyLewen and others added 5 commits June 10, 2024 15:53

docs: add info to docs + add link

ec412a3

docs: some tweaks

d2a8b23

Signed-off-by: knqyf263 <[email protected]>

chore: simplify an error

84c2db8

Signed-off-by: knqyf263 <[email protected]>

Merge branch 'main' into refactor/vendor-severity-missing-log

6c65774

docs: add SeveritySource

ceab109

Signed-off-by: knqyf263 <[email protected]>

knqyf263 approved these changes Jun 19, 2024

View reviewed changes

knqyf263 enabled auto-merge June 19, 2024 11:27

knqyf263 added this pull request to the merge queue Jun 19, 2024

Merged via the queue into aquasecurity:main with commit dfe757e Jun 19, 2024
17 checks passed

DmitriyLewen deleted the refactor/vendor-severity-missing-log branch June 20, 2024 02:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: add warning if severity not from vendor (or NVD or GH) is used #6726

refactor: add warning if severity not from vendor (or NVD or GH) is used #6726

DmitriyLewen commented May 20, 2024 •

edited

Loading

knqyf263 May 28, 2024

knqyf263 May 28, 2024

DmitriyLewen May 28, 2024

knqyf263 May 29, 2024

DmitriyLewen May 29, 2024 •

edited

Loading

knqyf263 Jun 3, 2024

DmitriyLewen Jun 3, 2024

knqyf263 Jun 3, 2024

DmitriyLewen Jun 3, 2024 •

edited

Loading

knqyf263 Jun 3, 2024

DmitriyLewen Jun 4, 2024

knqyf263 Jun 10, 2024

DmitriyLewen Jun 10, 2024

DmitriyLewen Jun 10, 2024

	log.Warn("Vendor and NVD don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", log.String("CVE", vulnID))
	log.Warn("Vendor and NVD don't have severity level. Severity from another vendor is used (see `VendorSeverity` in `json` format).", log.String("vulnerability-id", vulnID))

refactor: add warning if severity not from vendor (or NVD or GH) is used #6726

refactor: add warning if severity not from vendor (or NVD or GH) is used #6726

Conversation

DmitriyLewen commented May 20, 2024 • edited Loading

Description

Related issues

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyLewen May 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyLewen Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyLewen commented May 20, 2024 •

edited

Loading

DmitriyLewen May 29, 2024 •

edited

Loading

DmitriyLewen Jun 3, 2024 •

edited

Loading