cvss: bug fixes

[hdonnay]

Fixes for #1230.

Thanks @pandatix for the concise bug report!

[codecov]

Codecov Report

Attention: Patch coverage is 76.35135% with 35 lines in your changes are missing coverage. Please review.

Project coverage is 56.28%. Comparing base (0831b93) to head (34c3319).
Report is 2 commits behind head on main.

Files	Patch %	Lines
toolkit/types/cvss/cvss.go	84.00%	9 Missing and 3 partials ⚠️
toolkit/types/cvss/cvss_v3.go	70.37%	4 Missing and 4 partials ⚠️
toolkit/types/cvss/cvss_v4.go	63.63%	4 Missing and 4 partials ⚠️
toolkit/types/cvss/cvss_v2.go	65.00%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1232      +/-   ##
==========================================
+ Coverage   55.76%   56.28%   +0.52%     
==========================================
  Files         266      266              
  Lines       16759    16844      +85     
==========================================
+ Hits         9345     9481     +136     
+ Misses       6445     6400      -45     
+ Partials      969      963       -6     

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

[pandatix]

Followup here before the review, there are still some issues.

CVSS v2.0

For vector AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H the current code did not raise an error, despite the documentation (Table 13) define once one metric of a group is defined (!= "ND") the whole group must be part of the vector. (but is not really clear about it...). The expected behavior is to include IR and AR too and only validate vectors that contains them.

CVSS v4.0

For vector CVSS:4.0/AV:A/AC:L/AT:N/PR:H/UI:A/VC:L/VI:H/VA:N/SC:N/SA:N/S:X the implementation did not raise any error despite it lacks the mandatory metric SI, as of the CVSS v4.0 specification Table 23.

[hdonnay]

For vector AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H the current code did not raise an error, despite the documentation (Table 13) define once one metric of a group is defined (!= "ND") the whole group must be part of the vector. (but is not really clear about it...). The expected behavior is to include IR and AR too and only validate vectors that contains them.

Oh, I see what you're saying. That is indeed a very indirect way to specify that behavior.

For vector CVSS:4.0/AV:A/AC:L/AT:N/PR:H/UI:A/VC:L/VI:H/VA:N/SC:N/SA:N/S:X the implementation did not raise any error despite it lacks the mandatory metric SI, as of the CVSS v4.0 specification Table 23.

Oof, yeah. Missed doing that validation.

[pandatix]

CVSS v2.0

One might also say that the environmental group implies the temporal group, but that's quite annoying and the standard should say that if it wants to say that.
That's not true, the environmental group does not imply the temporal one to be included, but I agree it should have been clarified. At least we hope this has been solved in CVSS v4.0 :)

For vector AV:A/AC:L/Au:N/C:C/I:C/A:C/CDP:H/TD:H/CR:H the current code does not raise an error at parsing despite it does not contain IR and AR both mandatory once one of the group is provided and != ND.

CVSS v4.0

For vector CVSS:4.0/AV:A/AC:L/AT:N/PR:H/UI:A/VC:LN/VI:L/VA:N/SC:N/SI:N/SA:N, the current code does not raise an error at parsing despite VC has invalid value LN. Most likely due to L and N being both valid separately.

[crozzy]

LGTM just one comment

[hdonnay]

/fast-forward

[pandatix]

Hey, there are still issues with CVSS v2.0 and v4.0 implementations.
Differential fuzzing did not highlighted other defects in CVSS v3 implementation.

Notice that in v4.0 implementation you export metrics sets to X (Undefined) in the input vector.
For simplicity (there are a lot of metrics in v4.0) I would recommend not to export it if set to X, but this is not an implementation issue 😄

CVSS v2.0

Parsing vector AV:A/AC:L/Au:N/C:C/I:C/A:C/E:C/RL:F/RC:C does not return an error despite metrics E and RL have no valid value C and F (respectively).

From spec Table 13 :

E:[U,POC,F,H,ND]/RL:[OF,TF,W,U,ND]/RC:[UC,UR,C,ND]

CVSS v4.0

1: Parsing vector CVSS:4.0/AV:A/AC:L/AT:N/PR:H/UI:A/VC:L/VI:L/VA:N/SC:N/SI:N/SA:N/U:a does not return an error despite metrics does not accept lowercase values AND metric U does not have a valide metric a (even upercased).

From spec Table 23 :

Provider Urgency (U) [X,Clear,Green,Amber,Red]

2: Vector CVSS:4.0/AV:A/AC:L/AT:N/PR:L/UI:A/VC:L/VI:L/VA:N/SC:N/SI:N/SA:H/CR:X has a score of 4.1 but the implementation return 4.7.

[pandatix]

@hdonnay up :)