cvss: add package for dealing with CVSS scores

[hdonnay]

This adds a package for handling CVSS versions 2, 3.0, 3.1, and 4. The envisioned used-case is for calculating scores and canonicalizing vector strings.

[codecov]

Codecov Report

Attention: 183 lines in your changes are missing coverage. Please review.

Comparison is base (95414ad) 53.33% compared to head (e0d5be9) 54.31%.

Files	Patch %	Lines
toolkit/types/cvss/cvss_v2.go	57.03%	48 Missing and 7 partials ⚠️
toolkit/types/cvss/cvss.go	78.87%	24 Missing and 6 partials ⚠️
toolkit/types/cvss/cvss_v4_score.go	87.22%	24 Missing and 5 partials ⚠️
toolkit/types/cvss/cvss_v3_score.go	69.23%	21 Missing and 7 partials ⚠️
toolkit/types/cvss/cvss_v4.go	73.68%	17 Missing and 3 partials ⚠️
toolkit/types/cvss/cvss_v3.go	78.37%	12 Missing and 4 partials ⚠️
toolkit/types/cvss/cvss_v2_score.go	83.33%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
+ Coverage   53.33%   54.31%   +0.97%     
==========================================
  Files         224      231       +7     
  Lines       17166    17934     +768     
==========================================
+ Hits         9156     9741     +585     
- Misses       7170     7319     +149     
- Partials      840      874      +34     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hdonnay]

#1144 came in shortly after I sent this.

Here's the criticism of the quick-n-dirty CVSS support in the osv package, so I can make sure they're addressed:

• CVSS v2.0 metric ordering is not respected, invalid vectors goes through without error
• mandatory metrics must be part of the vector, at least once at most once
• non-mandatory metrics could be part of the vector, at most once
• obvious invalid CVSS v2.0 score computation

In my defence for the last one, there's no specified qualitative mapping for v2 so I felt OK getting in the ballpark.

---

@pandatix If I can harass you over here, I'm unclear on how/if the v3/v4 environmental scores are combined with the base scores. Are the base metrics just ignored if the environmental metrics are present?

[hdonnay]

I think this implementation is going to be generous with metric order on ingesting, seeing as the v4 spec is self-contradictory on this point.

[pandatix]

Hey, no problem, if I can help you :)

Yes the idea of an Environmental metric that finds its equivalent in the Base metrics group (for instance Attack Vector / AV and Modified Attack Vector / MAV), if the Modified is not specified in a vector string representation or if it is set to Undefined / X and its Base equivalent value is used for further calculations. If not, it is the value of the Base equivalent that is used.

So if MAV != X, take MAV, else take AV 🎉

[hdonnay]

Great, that makes sense. 👍

[hdonnay]

Going to make the decision to allow invalid orderings for vector parsing.

Not doing this would mean not being able to implement v4 to spec while using the published examples. I also think the erratum that makes the spec internally consistent will have to allow for parsing in both the currently specified order and the examples order.

[pandatix]

Hey, did you saw an invalid-ordered vector in the specification/examples/guide ?

We won't come back on our decision of fixing the CVSS v4 metric order as it is justified by the fact that an unfixed ordering would not enable providing a valid regular expression for use (for front-end/back-end validation, as achieved in the CVE Schema). If you try you will end up with n! combinations and with 32 metrics the regular expression would be terrabytes long.
This is a known problem with the CVSS v3 spec that we adressed through v4.
To properly implement CVSS v4 you must validate ordering.

[hdonnay]

Yeah, as I pointed out on RedHatProductSecurity/cvss-v4-calculator#44, all the examples are invalid and the calculator is implemented with the invalid order.

I think having the order specified is the correct choice, but unless all the examples are changed and all existing implementations are changed, any real implementation is going to need to accept both the specified order (VC/SC/VI/SI/VA/SA) and the observed order (VC/VI/VA/SC/SI/SA). Implementing alternative ordering for just these metrics is a lot of work.

Given that this package isn't meant for validation and only emits valid vectors, I think being liberal in accepted input is fine.

[pandatix]

The RedHat calculator is the official calculator, and you can easily check that here. As I already answered on your issue, the ordering problem lies in the specification and will be fixed soon.
This official calculator is valid.
An implementation that validates VC / SC / VI / SI / VA / SA is not and won't be compliant as of CVSS v4.0 Section 7.

The examples in the CVSS v4.0 Examples are valid, so could you please highlight why you don't think so ?

I don't recommend validating invalid vectors i.e. the ones that does not fully comply with the CVSS v4.0 specification (Section 7 requires you to not do it too) as it may propagate invalid values through the supply-chain, mostly for safety reasons, and for compliance sake.
As long as I have no power on this PR, I can't decide on anything, I can't tell you to have a Zero-Trust approach on your data feeds, but I don't recommend you to begin implementing the standard so poorly... That is the reason why I wrote a Go module for CVSS 😀

EDIT:

This package's implementation is built to mirror the Javascript implementation rather than as specified.

The RedHat official calculator is a direct implementation of the specification, this comment is completely invalid and misleading to the developers who will read this code.

EDIT 2:

I put this proposal implementation under the differential fuzzing of github.com/pandatix/go-cvss/differential and found multiple issues/vulnerabilities:

• CVSS v2.0 parsing /!\ nil pointer dereference /!\ on the call of cvss.(*V2).UnmarshalText and specifically at cvss.go:164
• CVSS v2.0 and CVSS v3.x misunderstanding of Base and Environmental score differences. What I explained sooner applies to the CVSS v4.0 specification, but for previous ones the formulas are different and have different behaviors. To ensure an upward provider do not pollute (Zero-Trust approach) the CVSS v2.0 and v3 data quay is ingesting you should only use the base metrics.
• CVSS v4.0 parsing does not properly check metric ordering as defined in the CVSS v4.0 specification with Errata announced here as it validates Vx/Sx metrics in the order VC / SC / VI / SI / VA / SA.
• CVSS v4.0 score computation is invalid. For instance the vector CVSS:4.0/AV:A/AC:H/AT:N/PR:H/UI:A/VC:L/VI:L/VA:L/SC:N/SI:N/SA:N/E:U has a CVSS-BT score of 0.1 according to the official calculator but this implementation produces NaN, which is not compliant with Section 8 (0.0 <= score <= 10.0). This invalid value would propagate through the API and quay's ecosystem and will unexpectedly break things.

Implementing CVSS is not as easy as it looks like, even with all the efforts we put into it 😉
I maintain you should reconsider #1144.

[hdonnay]

I see there's a new revision of the spec, so I've updated the code. I think it's pretty impolite to describe implementing the specification as written as "implementing the standard so poorly."

I stand by my comment about the javascript implementation, as the weights and specific ordering of "subvectors" can only be read out of the javascript implementation; they're not in the specification proper. Getting the same "highest-score" vector requires looking at the javascript implementation because it's not specified at all.

I don't understand what you're saying about "upward provider pollution." We're always consuming data from vendors so if they're adding additional metrics, we want to make use of them.

[hdonnay]

/fast-forward