jeremylong / Open-Vulnerability-Project

Java libraries for working with available vulnerability data sources (GitHub Security Advisories, NVD, EPSS, CISA Known Exploited Vulnerabilities, etc.)
Apache License 2.0
107 stars 30 forks source link

Support for Common Vulnerability Scoring System (CVSS) version 4.0 #163

Closed nhumblot closed 2 weeks ago

nhumblot commented 3 weeks ago

Context

The CVSS 4.0 specification has been released. As this project supports CVSS 2 & 3.1 specifications, it would be a nice addition to support this new 4.0 version.

Implementing this feature would permit to fix https://github.com/jeremylong/DependencyCheck/issues/6734 by relying on this library.

I am volunteer to look to provide a PR on this topic if it is not a work in progress already. I am creating this issue to track it. Please feel free to share any comments or remarks regarding the expected implementation. :slightly_smiling_face:

Resources

malice00 commented 3 weeks ago

I think CVSS 4 just went live at the NVD -- vulnz stopped working for me and debug-output is giving me this error: Error processing NVD data com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "cvssMetricV40" (class io.github.jeremylong.openvulnerability.client.nvd.Metrics), not marked as ignorable (3 known properties: "cvssMetricV30", "cvssMetricV31", "cvssMetricV2"])

chadlwilson commented 3 weeks ago

At the very least, the library should use lenient parsing here. It's not good practice to have strict parsing when consuming some else's API. They have a reasonable assumption to be be able to add new fields without breaking clients.

chadlwilson commented 3 weeks ago

Created a PR to workaround the current problem which is breaking Vulnz and ODC for everyone.

Seems there is now:

aikebah commented 2 weeks ago

At the very least, the library should use lenient parsing here. It's not good practice to have strict parsing when consuming some else's API. They have a reasonable assumption to be be able to add new fields without breaking clients.

If they want to have the freedom to do so without "a new major" or "breaking consumers" they should not define "additional properties: false" in their schema - https://json-schema.org/understanding-json-schema/reference/object#additionalproperties

chadlwilson commented 2 weeks ago

@aikebah That's why they updated this a few weeks ago for this reason, I guess: https://nvd.nist.gov/general/news/api-cves-schema-restriction-update

Removed it from metrics in particular, which would allow their CVSS v4.0 changes to flow through without breaking things (albeit pretty late notice).

IMHO the analysis at #158 was wrong - the removal of additionalProperties: false means at the very least the parsing for those fields should become lenient.

Perhaps I should adjust my PR to only add the annotation to those 3 entities if we want to follow the schema exactly?

aikebah commented 2 weeks ago

@chadlwilson Looks at a quick read that the impact of the additionalProperties removal was indeed incorrectly assessed in #158 as the effect of it is making the strict parser lenient in the sense that the schema already specifies that properties other than those already mentioned are allowed, and therefor also accepted by a strict parser.

chadlwilson commented 2 weeks ago

Yeah, feel free to weigh in on #165 anyway. Happy to adjust/review the PR to only be lenient where the schema tells us to if we want to follow it directly.

aikebah commented 2 weeks ago

Perhaps I should adjust my PR to only add the annotation to those 3 entities if we want to follow the schema exactly?

Annotation may be needed in any case, as that annotation appears to be for the mapping and would be required for any schema that has 'additionalProperties: true' or leaves out the additionalProperties so that it defaults to true.

My read on that annotation is that it tells the mapper "if there is no corresponding field for any of the properties you encounter just continue to map just the properties that you can map" (not sure whether Jackson would automatically map 'lenient' when additionalProperties is explicitly or implicitly true)

edit: @chadlwilson Glancing over Jackson docs my impression is that JSON schema itself is not used in mapping configuration, so lenient mapping would need to be configured with the annotation for mapping any schema-type that does not explicitly forbid additional properties.

jeremylong commented 2 weeks ago

I'll release the library in the morning and then work on ODC.