Review CVSS score handling & reporting #118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For dependency-check: Clj-watson now recognizes that multiple CVSS versions can be populated for a single CVE. We now: - to be cautious, choose the highest base score across all CVSS versions - include the CVSS version with the score For github-advisory: The github-advisory only contains a single CVSS entry. Clj-watson now extracts the CVSS revision from the CVSS "vectorString", when available. For reports: - `json` & `edn` - now include the CVSS `:version` under `:cvss` - `stdout` - now includes version after score: `CVSS: <score> (version <cvss version>)` - `sarif` - added `cvss` with its `score`, `version` and `severity` under `properties`, this duplicates existing the (unfortunately named) `security-severity` which also holds the `score` - reworded slightly awkward summary message, ex: - old: Vulnerability identified as CVE-2022-4244 of score 7.5 and severity HIGH found. - new: Vulnerability CVE-2022-4244 with a score of 7.5 and severity of HIGH found. Out of scope: This change does not include support for deriving a CVSS score when it missing. This will be handled when we need it for decision making, like in clj-holmes#114. Closes clj-holmes#112
| (some-> vulnerability .getCvssV3 .getCvssData .getBaseSeverity str) | ||
| (catch Exception _ | ||
| nil))) | ||
| (defn ^:private base-score [cvss] |
There was a problem hiding this comment.
In the old code, each of these chains of calls is wrapped in try / catch -- is there any danger that the call chains can actually throw or was the old code overly defensive?
There was a problem hiding this comment.
My feeling was the old code was hiding potential issues.
I don't see why we should normally encounter an exception here (and didn't see a rationale in the git history).
If we learn otherwise, we can adapt and address.
There was a problem hiding this comment.
Fair enough. I wasn't sure whether those methods were declared to throw exceptions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For dependency-check:
Clj-watson now recognizes that multiple CVSS versions can be populated for a single CVE. We now:
For github-advisory:
The github-advisory only contains a single CVSS entry. Clj-watson now extracts the CVSS revision from the CVSS "vectorString", when available.
For reports:
json&edn- now include the CVSS:versionunder:cvssstdout- now includes version after score:CVSS: <score> (version <cvss version>)sarifcvsswith itsscore,versionandseverityunderproperties, this duplicates the existing (unfortunately named)security-severitywhich also holds thescoreOut of scope:
This change does not include support for deriving a CVSS score when it missing. This will be handled when we need it for decision making, like in #114.
Closes #112