ROX-12350: Detect CVE-2022-22978 #930
Conversation
RTann
added
the
generate-dumps-on-pr
|
Images are ready for the commit at d97d7e0. To use the images, use the tag |
|
@RTann: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
To be honest, from reading the problem description in the PR description, the solution seems non-obvious to me.
If I understand the mechanics correctly, we now detect a vulnerability that is attributed to spring_security in an image that uses the spring_security_web package by essentially duplicating the vuln in a manual entry. Isn't this very prone to not ageing well? What if the severity or some other metadata of the vuln is updated upstream?
Rather than having manual vuln entries, wouldn't a targeted "CPE override" be the feature we want? Another possible alternative: should we have a notion of "virtual packages"? I.e., if a vuln is reported to affect spring_security, shouldn't we treat it as affecting spring_security_*? From the referenced Snyk entry, it seems quite obvious that it indeed only affects spring_security_web, so a CPE with that seems the more precise option. However, in the general case, this seems to be the right thing to do, because there isn't even a spring_security package per se, just a lot of spring-security-* packages.
| for name := range names { | ||
| if knownSpringComponents.Contains(name) { | ||
| vendorSet.AddAll(knownSpringVendors...) | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
If performance is not a strong concern (it might be, though), you could also do
| for name := range names { | |
| if knownSpringComponents.Contains(name) { | |
| vendorSet.AddAll(knownSpringVendors...) | |
| break | |
| } | |
| } | |
| if !names.Intersect(knownSpringComponents).IsEmpty() { | |
| vendorSet.AddAll(knownSpringVendors...) | |
| } |
Or, to address the performance issue, maybe it would also be nice to add a Intersects(other Set) bool method to pkg/set (not in this PR, just as an idea).
| @@ -90,6 +120,15 @@ func getPossibleVendors(origins []string) set.StringSet { | |||
| if vendorSet.Cardinality() == 0 { | |||
| vendorSet.Add("apache") | |||
| } | |||
|
|
|||
| // Add Spring-specific vendors. | |||
There was a problem hiding this comment.
Does it make sense to do this only after adding "apache" based on nothing but hope? Isn't the knownSpringComponents check more precise than the "apache" default and therefore should suppress the latter?
There was a problem hiding this comment.
I guess another question would be, is it adequate to do this here even when vendorSet is non-empty (and wasn't populated via the apache fallback)?
|
Note: if this is time-sensitive and you share my view but deem those infeasible to implement in the timeframe, that's fine, just curious if you had considered those points. |
This PR ensures we can detect CVE-2022-22978. When inspecting an example Spring application, I found the
spring-securityJARs were named more than simplyspring-security. For example, there isspring-security-web. In NVD, the CPE only matchesspring-security, so I add a manual entry to ensure we can detect it. I followed Snyk's lead https://security.snyk.io/vuln/SNYK-JAVA-ORGSPRINGFRAMEWORKSECURITY-2833359 and attributed the vuln tospring-security-web.In this PR, I also try to future-proof and try to associate known Spring components to known vendors to ensure we can capture more Spring-related vulns.