-
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: workaround lack of CVSSv4 support with consistently lenient JSON parsing #165
Conversation
43c81e4
to
dd7f0f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This comment was marked as resolved.
This comment was marked as resolved.
I think we should only ignore properties on Metrics, Reference and CveItem. |
I'll update the objects to support 4 shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only ignore additional properties on the references, cve_item, and metrics.
...ility-clients/src/main/java/io/github/jeremylong/openvulnerability/client/epss/EpssItem.java
Show resolved
Hide resolved
...ility-clients/src/main/java/io/github/jeremylong/openvulnerability/client/epss/EpssItem.java
Show resolved
Hide resolved
...lity-clients/src/main/java/io/github/jeremylong/openvulnerability/client/kev/KevCatalog.java
Show resolved
Hide resolved
...lity-clients/src/main/java/io/github/jeremylong/openvulnerability/client/kev/KevCatalog.java
Show resolved
Hide resolved
...ability-clients/src/main/java/io/github/jeremylong/openvulnerability/client/kev/KevItem.java
Show resolved
Hide resolved
...y-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/VendorComment.java
Outdated
Show resolved
Hide resolved
...bility-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/Weakness.java
Outdated
Show resolved
Hide resolved
...bility-clients/src/main/java/io/github/jeremylong/openvulnerability/client/nvd/Weakness.java
Outdated
Show resolved
Hide resolved
vulnz/src/main/java/io/github/jeremylong/vulnz/cli/model/BasicOutput.java
Outdated
Show resolved
Hide resolved
vulnz/src/main/java/io/github/jeremylong/vulnz/cli/model/BasicOutput.java
Outdated
Show resolved
Hide resolved
@jeremylong as indicated in the thread on #163 any schema-type that does not include "additionalProperties: false" is by definition communicating that additional properties may be added without violating the JSON scheme (JSON is by-design lenient, contrary to XML XSDs, where this lenient behaviour needs to be explicitly opted in to by adding an So for any type that represents a JSON structure for which the schema does not explicitly forbid extension ( |
This comment was marked as resolved.
This comment was marked as resolved.
330e38d
to
e63fafe
Compare
As @aikebah enumerates, there are more than these three to fix as some others were strict when they should not be, allowing for possible future bugs. However, I have updated and rebased to remove Summary for non NVD:
Summary for NVD
Let me know :-) |
e63fafe
to
bcac95e
Compare
Hiya @jeremylong - would it be possible to focus on trying to get a fix out for ODC independently of/prior to supporting CVSS v4? Right now more people are probably blocked by having ODC fall over rather than worrying about CVSS v4, and it might be wise to decouple the two. |
bcac95e
to
f3a0393
Compare
I wanted to add this one last - after I added support for CVSS v4. |
Ignore additional properties for all models where they do NOT have a JSON schema that declares "additionalProperties: false" for the element/node.
f3a0393
to
0f3f9f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
OK, rebased. I was afraid that the CVSS v4 support might cause other unexpected problems (esp if they change their API again) and we are in a position we we have no working version of dependency check for longer. :-( |
As noted in jeremylong/DependencyCheck#6747 NVD have added
cvssMetricV40
to their API which breaks this library as it is using strict deserialisation in many places it shouldnt.This change helps workaround/fix jeremylong/DependencyCheck#6747 and jeremylong/DependencyCheck#6746 in the short term, before #163 is implemented.