-
-
Notifications
You must be signed in to change notification settings - Fork 699
Add CVSSv4 support to Dependency-Track #5456
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
base: master
Are you sure you want to change the base?
Add CVSSv4 support to Dependency-Track #5456
Conversation
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…it does have optional environmental and threat scores. Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…t exposing CVSSv4 base score Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…s to be on par with Trivy v0.67. Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…ilities Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…oadTask Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…lysisTask Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
…ests Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
a16904e to
cda9018
Compare
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
8753218 to
f7dc16c
Compare
|
Well that's new. I haven't seen that test fail in my local test runs yet. I'll have a look to see if I can reproduce... any info on if it's just a bit brittle? EDIT: From what I can tell it seems we just found a particularly busy Actions runner. The test takes between 5 and 6 seconds to run on my local machine (AMD Ryzen 9 9900X, 64 GB RAM). I could increase the timeout for this test to 15 seconds to make it less likely for this to re-occur. Not sure if that's desirable though. |
IMHO you can do that as long as it's not excessive. I have done so myself in other PRs to mitigate some flaky tests that were clearly failing because of low resources in the CI environment. |
…laky Signed-off-by: Tobias Gies <tobias@tobiasgies.de>
Alright, thank you. In that case: Extended timeout to 20s to hopefully alleviate the flakiness. |
| @Persistent | ||
| @Column(name = "CVSSV4VECTOR") | ||
| @JsonDeserialize(using = TrimmedStringDeserializer.class) | ||
| @Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS_PLUS, message = "The CVSSv4 Vector may only contain printable characters") |
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.
Maybe we should do here more strict validation, accept only valid CVSSv4 vectors
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.
I agree in principle, I just copy/pasted the validation code for the other CVSS vector versions for the moment. However I think this might be duplicate work - the org.metaeffekt.core:ae-security package contains stricter validation logic in CvssVector.parseVector. Any invalid vector will cause the method to return null instead of a CvssVector instance, causing the data to be thrown out.
With that in mind, do you still think additional validation is needed in this place?
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.
Yes I know that for every other places related to CVSS we have PRINTABLE_CHARS_PLUS.
Regarding to
Any invalid vector will cause the method to return null instead of a CvssVector instance, causing the data to be thrown out.
validation in CvssVector.parseVector is also very simple, regex is:
private static final Pattern CVSS_PATTERN = Pattern.compile("CVSS:\\d+\\.?\\d?");
For instance, I am able to create such internal Vulnerability in DependencyTrack 4.13.6(note that i was able to put invalid cvss 4.0 vector into cvss 2.0)

From my experience strict validation should be present from the begining of the feature because then data that normaly will be rejected(because validation) are living in db and can cause a problems
Second thing is that OWASP product should be secure as much as possible, because this is OWASP mission at the end,
how can we teach people to write application according to ASVS 4.X when we not do the same.
On the other hand previous version of cvss was written in a way that is used in PR, making difference here can cause problems that i am not aware of, so I agree with that maybe it can be addressed later(I can do that)
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.
It might make sense to look into creating a custom validator, like we have for cron expressions for example.
I do agree that adding that for existing CVSS types might cause issues that would need proper data migration first. But for everything we add going forward, it makes sense to be more defensive as to what we accept.
| private void normalizeCpeData(final Connection connection) throws SQLException { | ||
| try (final Statement statement = connection.createStatement()) { | ||
| LOGGER.info("Adding CVSSv4 columns to \"VULNERABILITY\""); | ||
| statement.execute(/* language=SQL */ """ | ||
| ALTER TABLE "VULNERABILITY" | ||
| ADD COLUMN "CVSSV4BASESCORE" numeric, | ||
| ADD COLUMN "CVSSV4THREATSCORE" numeric, | ||
| ADD COLUMN "CVSSV4ENVIRONMENTALSCORE" numeric, | ||
| ADD COLUMN "CVSSV4VECTOR" varchar(255); | ||
| """); | ||
| } | ||
| } |
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.
A migration shouldn't be necessary, the persistence framework should add columns itself.
Description
This adds support for CVSSv4 scores to Dependency-Track. Scores are stored in the database, will be returned as part of the relevant HTTP API resources following existing conventions, can be updated via the REST API, and will be processed by parsers for most vulnerability sources. They will also be preferred over CVSSv3 and CVSSv2 scores when determining the severity of a vulnerability.
Addressed Issue
fixes #4707
Additional Details
CVSSv4 handling not implemented in certain parsers
The following parsers do not have handling for CVSSv4 scores added by this PR, since the APIs they are based on are commercial offerings that I do not have access to:
Only backend work completed for now
This PR obviously only addresses the server / API side of implementing CVSSv4 support. I am planning to work on support in the frontend next, but wanted to put my work on this repository out there first. I hope to get some feedback on this so I don't run in the wrong direction with any frontend changes.
Trivy protobufs updated
To be able to process CVSSv4 scores supplied by trivy, I have updated the protobuf files stored in this repo to the state of their
release/v0.67branch. I have kept the customizations / changes to the headers of the protobuf files the same, though. For the remainder of each protobuf file, I chose easier copy/paste-ability in the future over keeping the diff small. I hope that's okay.Checklist