Consider CVSS2 and CVSS4 scores

lread commented 2 months ago

Background

The CVSS score is a number from 0.0 to 10.0 that conveys the relative severity of a vulnerability.

Currently

clj-watson looks only at the CVSS3 score.

But..

Some (assumedly older?) vulnerabilities only have the CVSS2 score populated. Take for example: https://nvd.nist.gov/vuln/detail/CVE-2014-3577

So...

We probably also want to look at and report CVSS2 scores. CVSS2 has a slightly different score->severity. (CVSS2 does not have the None and Critical severities)

What about CVSS4?

I'm not sure. These have the same score->severity mapping as CVSS3. Here's a CVE I found that has CVSS4 scores: https://nvd.nist.gov/vuln/detail/CVE-2024-7922 (we'll never get this one for our deps, but I expect we could get one with similar scoring populated).

Next Steps

Maybe we should better understand what these various scores mean, which ones we should be reporting, and if they are comparable. Intuitively, the "base" score (which we are currently reporting) seems to be a good score to report.

What does nvd-clojure do? Unlike clj-watson, it relies on DependencyCheck to generate its reports. But it does provide a summary of findings, and for that, it does look at both CVSS2 and CVSS3.

lread commented 2 months ago

Initial thought: I don't think we have to be terribly pedantic about this. But I think we should:

always share a meaningful CVSS if it is available
give some indication of its version

seancorfield commented 2 months ago

Can a vulnerability have multiple scores (from different CVSS versions)? If so, would we want to pick the highest? I'm thinking of #114 here.

lread commented 2 months ago

Yes, there a single CVE can have scores from different versions.

My initial research suggests I should favor CVSS4, then CVSS3, then CVSS2 - the idea being that the latest version is the most accurate.

Within each CVSS version, there are also multiple scores; my initial thinking is that we go with the "base" score. I had a peek in the DependencyCheck h2 db and saw only five rows with null base scores across all CVSS versions. So that seems promising.

I'll look into what DependencyCheck does to satisfy the equivalent of #114.

lread commented 2 months ago

Ok, I've dug into CVEs with both CSSV3 and CSSV4 scores. These can vary wildly. For one example https://nvd.nist.gov/vuln/detail/CVE-2023-38524 DependencyCheck db has a CSSV4 base score of 2.0 and a CSSV3 base score of 7.8. We risk downplaying a severity if we were to show 2.0.

Therefore, I think your idea is better, @seancorfield, that we should pick the highest score if we are to only show one score. Or maybe we should show all available base scores in a detailed views, but when making any decisions, or summarizing, use the highest base score.

coyotesqrl commented 2 months ago

I think showing all makes a ton of sense, and when thresholds are in place, they should fail if the maximum value of any cve's score is over the line. If it turns out to be something the users are okay accepting, they can always whitelist with the xml approach.

lread commented 2 months ago

I think showing all makes a ton of sense

Thanks for chiming in @coyotesqrl! There are other scores, but I am only planning on showing the "base" score for now. I think you suggest we show all the available "base" scores in detailed reports. I think I agree. (The user can bring up the CVE on the web to get other scores).

I'm guessing most folks are interested in getting an idea of what vulnerable deps they are using, how bad it might be, learning about potential remediations, applying them, and then moving on with their lives. So, I'm hesitant to include all the data that is available.

I have other places where I'm going to show severity in summary views. See #87 & #113 In those cases, I think showing the highest "base" severity is the more cautious approach.

seancorfield commented 2 months ago

Interesting... I didn't expect quite that much variation!

Yes, definitely picking the highest score make sense to me, esp. if we're going to have a "fail threshold".

lread commented 2 months ago

Ok, I think we are getting close to a strategy here!

CVSS in Summaries and Decisions

We agree that for summary reporting (#87, #113) and decisions (#114), we should use a CVE's highest available base score (a number from 0 to 10) or, when applicable, the most severe base severity (low, medium, high, critical (and potentially unknown will check on that)). (The base severity is directly derived from the score).

A nuance here is that CVSS2 doesn't have critical severity. So a CVSS2 score of 9.2 will translate to high. And a CVSS3 score of 9.0 will translate to critical. I think it is "good enough" that when we choose a CVSS2 score (because it is higher) to show its actual CVSS2 severity, in this case, high.

CVSS in Detailed Reporting

I don't see a clear decision here yet. Clj-watson is currently reporting the CVSS2 base score and severity like so:

json

Assuming a fix for #116 is in place: Under "vulnerabilities":->"advisory":

  "cvss": { "score": 7.5},
  "severity": "HIGH"

edn

Under :vulnerabilities->:advisory:

  :cvss {:score 7.5},
  :severity "HIGH",

stdout

...
SEVERITY: HIGH
IDENTIFIERS: CVE-2022-4244 
CVSS: 7.5
...

stdout-simple

Does not emit severity nor cvss.

sarif

The Sarif report embeds cvss and severity in the shortDescription and under properties.

...
              "shortDescription": {
                "text": "Vulnerability identified as CVE-2022-4244 of score 7.5 and severity HIGH found."
              },
...
              "properties": {
                "security-severity": "7.5"
              },

Proposal

For detailed reports, it is probably "good enough" to include the CVSS version we are using. The current DependencyCheck db has: 2.0, 3.0, 3.1 and 4.0. There are only 5 rows in the db where no CVSS version is set, and in those cases, the CVSS score is not set either.

The docs can mention that we pick the highest CVSS score we find.

This might look like:

json

  "cvss": { "score": 7.5
            "version": 2.0},
  "severity": "HIGH"

edn

  :cvss {:score 7.5
         :version 2.0},
  :severity "HIGH",

stdout

...
SEVERITY: HIGH
IDENTIFIERS: CVE-2022-4244 
CVSS: 7.5 (version 2.0)
...

stdout-simple

n/a

sarif

...
              "shortDescription": {
                "text": "Vulnerability identified as CVE-2022-4244 of score 7.5 and severity HIGH found."
              },
...
              "properties": {
                "security-severity": "7.5"
                "cvss-version": "2.0"
              },

Whatcha think?

coyotesqrl commented 2 months ago

For what little worth my vote has, I like it.

It might not be terrible - if it's not breaking an output contract - to add a parameter for more verbose/all-cvss/something output that would maybe look something like this:

json

"cvss": [
  {
    "score": 7.5,
    "version": 2.0
  },
  {
    "score": 6.7,
    "version": 3.0
  }
],
"severity": "HIGH"

edn

:cvss [{:score   7.5
        :version 2.0}
       {:score   6.7
        :version 3.0}],
:severity "HIGH",

stdout

...
SEVERITY: HIGH
IDENTIFIERS: CVE-2022-4244 
CVSS: 7.5 (version 2.0), 6.7 (version 3.0)
...

sarif

But I have no idea what would reasonable for SARIF output.

lread commented 2 months ago

Thanks @coyotesqrl. Your proposed change would be technically a breaking one. That said, I'm pretty sure that if we were starting today, we would have gone with something like it.

But note that you've not tied severity to the cvss score and if we were going to show all cvss base scores, we would have to also include it, ex:

:cvss [{:score 9.2 :version 2.0 :severity "HIGH"}
       {:score 9.0 :version 3.0 :severity "CRITICAL"}]

If we really want to include all base scores, maybe keep our original :cvss and :severity, and these become the score we've picked. And add a new :cvss-scores:

:cvss {:score 9.2}
:severity "HIGH"
:cvss-scores [{:score 9.2 :version 2.0 :severity "HIGH"}
              {:score 9.0 :version 3.0 :severity "CRITICAL"}]

Does showing all cvss base cores help your use case in some way? (Remembering that we omit lots of other available data from detailed reports).

coyotesqrl commented 2 months ago

Showing all scores is irrelevant to me.

I agree that your original proposal, showing the highest base score, is the best. I just thought there might be someone out there who'd want to get a fuller report. But the user can always go to the CVE report to get that extra information if they're so inclined.

seancorfield commented 2 months ago

For now, I'd be happy with just adding version to the cvss in this ticket (and to stdout).

If someone wants all the scores in the JSON/EDN, we can create a new issue for that (and go with @lread proposal to add cvss-scores perhaps).

lread commented 2 months ago

Thanks guys, the discussion has helped, I think we have a strategy now!

lread commented 2 months ago

I was focusing on dependency-check.

For github-advisory we seem to have:

a single cvss score
it looks like its vectorString contains the CVSS version. An example: CVSS:3.1/AV:N/AC:H/PR:L/UI:R/S:U/C:H/I:H/A:H
its severity uses Moderate where NIST uses Medium
the severity does not seem to always be derived from the the cvss score, for example, I see cases of score of 0.0 with vectorString of nil and a severity of MODERATE or LOW

I think this is all probably OK-ish:

We'll extract the cssv version when we can.
When the score is 0.0 (or missing if it ever is) we'll report it as such. But I suppose we could derive a more real score from the severity (using the NIST score->severity mappings) when we need to make decisions, like for #114. To be cautious, we'd choose the upper bound. When we are doing this we'll want to share this with the user (but that's a topic for #114).

lread commented 2 months ago

Ref for github advisory db cvss: https://docs.github.com/en/code-security/security-advisories/working-with-global-security-advisories-from-the-github-advisory-database/about-the-github-advisory-database#about-cvss-levels

clj-holmes / clj-watson