Skip to content

Commit 00bc660

Browse files
surlimanuelleduc
surli
and
manuelleduc
committed
XWIKI-23646: Security vulnerability application improvments (#4722)
 * Allow computation of CVSS v4 * Only limit displays of vulnerabilities based on CVE ID and no longer on max severity * Support multiple severities and prioritize them --------- Co-authored-by: Manuel Leduc <[email protected]> (cherry picked from commit c3b71ba)
1 parent 7c08224 commit 00bc660

File tree

14 files changed

+459
-23
lines changed

14 files changed

+459
-23
lines changed

‎pom.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@
6767
<netty.version>4.2.7.Final</netty.version>
6868
<hibernate.version>5.6.15.Final</hibernate.version>
6969
<dockerJava.version>3.5.0</dockerJava.version>
70-
<cvssCalculator.version>1.4.3</cvssCalculator.version>
70+
<!-- Used for CVSS score calculation -->
71+
<metaeffekt.version>0.147.0</metaeffekt.version>
7172
<solr.major.version>9</solr.major.version>
7273
<solr.version>9.4.1</solr.version>
7374
<lucene.version>9.8.0</lucene.version>

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-index/pom.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@
5959
<version>${project.version}</version>
6060
</dependency>
6161
<dependency>
62-
<groupId>us.springett</groupId>
63-
<artifactId>cvss-calculator</artifactId>
64-
<version>${cvssCalculator.version}</version>
62+
<groupId>org.metaeffekt.core</groupId>
63+
<artifactId>ae-security</artifactId>
64+
<version>${metaeffekt.version}</version>
6565
</dependency>
6666

6767
<!-- Test dependencies -->

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-index/src/main/java/org/xwiki/extension/index/security/SecurityVulnerabilityDescriptor.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@
2525
import org.apache.commons.lang3.StringUtils;
2626
import org.apache.commons.lang3.builder.EqualsBuilder;
2727
import org.apache.commons.lang3.builder.HashCodeBuilder;
28+
import org.metaeffekt.core.security.cvss.CvssVector;
2829
import org.xwiki.extension.version.Version;
2930
import org.xwiki.text.XWikiToStringBuilder;
3031

31-
import us.springett.cvss.Cvss;
32-
3332
/**
3433
* An individual security vulnerability descriptor.
3534
*
@@ -98,7 +97,7 @@ public String getURL()
9897
public SecurityVulnerabilityDescriptor setSeverityScore(String vector)
9998
{
10099
if (StringUtils.isNotEmpty(vector)) {
101-
this.score = Cvss.fromVector(vector).calculateScore().getBaseScore();
100+
this.score = CvssVector.parseVector(vector).getBaseScore();
102101
}
103102
return this;
104103
}

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-api/src/main/java/org/xwiki/extension/security/internal/ExtensionSecuritySolrClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ private static void initFilter(SolrQuery solrQuery)
170170
}
171171
// Only include extensions with a computed CVSS score, meaning that they have at least one known security
172172
// vulnerability.
173-
solrQuery.addFilterQuery(String.format("%s:{0 TO 10]", SECURITY_MAX_CVSS));
173+
solrQuery.addFilterQuery(String.format("%s:[* TO *]", SECURITY_CVE_ID));
174174
solrQuery.addFilterQuery(String.format("(%s:[* TO *] OR %s:true)",
175175
FIELD_INSTALLED_NAMESPACES, IS_CORE_EXTENSION));
176176
}

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-api/src/main/java/org/xwiki/extension/security/internal/SolrToLiveDataEntryMapper.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,11 @@ private String buildFixVersion(SolrDocument doc)
227227

228228
private Double buildMaxCVSS(SolrDocument doc)
229229
{
230-
return this.solrUtils.get(SECURITY_MAX_CVSS, doc);
230+
Double result = this.solrUtils.get(SECURITY_MAX_CVSS, doc);
231+
if (result == null) {
232+
result = 0.0;
233+
}
234+
return result;
231235
}
232236

233237
private String buildExtensionId(SolrDocument doc)

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-api/src/test/java/org/xwiki/extension/security/internal/ExtensionSecuritySolrClientTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ void getExtensionsCount() throws Exception
8989
assertEquals(42, this.solrClient.getVulnerableExtensionsCount());
9090

9191
SolrQuery params = new SolrQuery();
92-
params.addFilterQuery(String.format("%s:{0 TO 10]", SECURITY_MAX_CVSS));
92+
params.addFilterQuery(SECURITY_CVE_ID + ":[* TO *]");
9393
params.addFilterQuery(getNamespaceFilterQuery());
9494
params.addFilterQuery(String.format("%s:false", IS_REVIEWED_SAFE));
9595
verify(this.extensionIndexStore)
@@ -111,7 +111,7 @@ void solrQuery() throws Exception
111111
this.solrClient.solrQuery(liveDataQuery);
112112

113113
SolrQuery params = new SolrQuery();
114-
params.addFilterQuery(SECURITY_MAX_CVSS + ":{0 TO 10]");
114+
params.addFilterQuery(SECURITY_CVE_ID + ":[* TO *]");
115115
params.addFilterQuery(IS_CORE_EXTENSION + ":false");
116116
params.addFilterQuery(getNamespaceFilterQuery());
117117

@@ -138,7 +138,7 @@ void solrQueryIsFromEnvironment() throws Exception
138138
this.solrClient.solrQuery(liveDataQuery);
139139

140140
SolrQuery params = new SolrQuery();
141-
params.addFilterQuery(SECURITY_MAX_CVSS + ":{0 TO 10]");
141+
params.addFilterQuery(SECURITY_CVE_ID + ":[* TO *]");
142142
params.addFilterQuery(IS_CORE_EXTENSION + ":true");
143143
params.addFilterQuery(getNamespaceFilterQuery());
144144

@@ -168,7 +168,7 @@ void filterByCVEId() throws Exception
168168

169169
SolrQuery expectedSolrQuery = new SolrQuery();
170170
expectedSolrQuery.addFilterQuery(String.format("%s:*test*", SECURITY_CVE_ID));
171-
expectedSolrQuery.addFilterQuery(SECURITY_MAX_CVSS + ":{0 TO 10]");
171+
expectedSolrQuery.addFilterQuery(SECURITY_CVE_ID + ":[* TO *]");
172172
expectedSolrQuery.addFilterQuery(getNamespaceFilterQuery());
173173
expectedSolrQuery.addFilterQuery(IS_CORE_EXTENSION + ":false");
174174

@@ -200,7 +200,7 @@ void filterByMaxCVSS(String value, String convertedValue) throws Exception
200200

201201
SolrQuery expectedSolrQuery = new SolrQuery();
202202
expectedSolrQuery.addFilterQuery(String.format("%s:[%s TO 10]", SECURITY_MAX_CVSS, convertedValue));
203-
expectedSolrQuery.addFilterQuery(SECURITY_MAX_CVSS + ":{0 TO 10]");
203+
expectedSolrQuery.addFilterQuery(SECURITY_CVE_ID + ":[* TO *]");
204204
expectedSolrQuery.addFilterQuery(getNamespaceFilterQuery());
205205
expectedSolrQuery.addFilterQuery(IS_CORE_EXTENSION + ":false");
206206

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-index/src/main/java/org/xwiki/extension/security/internal/analyzer/VulnerabilityIndexer.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ public boolean update(Extension extension, ExtensionSecurityAnalysisResult analy
8989
.anyMatch(vulnerability -> {
9090
String vulnerabilityId = vulnerability.getId();
9191
Set<String> aliases = vulnerability.getAliases();
92-
return vulnerability.getScore() > 0
93-
&& !cveiDs.contains(vulnerabilityId)
92+
return !cveiDs.contains(vulnerabilityId)
9493
// To avoid raising a notification for a vulnerability that was already notified with another id
9594
&& cveiDs.stream().noneMatch(aliases::contains)
9695
&& isNotSafe(reviewsMap, vulnerabilityId);

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-index/src/main/java/org/xwiki/extension/security/internal/analyzer/osv/OsvResponseAnalyzer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ private Optional<SecurityVulnerabilityDescriptor> convert(VulnObject vulnObject,
9292
.setId(id)
9393
.setAliases(resolveAliases(vulnObject, id))
9494
.setURL(vulnObject.getMainURL())
95-
.setSeverityScore(vulnObject.getSeverityCCSV3())
95+
.setSeverityScore(vulnObject.getSeverityCVSS())
9696
.setFixVersion(vulnObject.getMaxFixVersion(currentVersion).orElse(null)));
9797
}
9898

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-index/src/main/java/org/xwiki/extension/security/internal/analyzer/osv/model/response/VulnObject.java‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
*/
3838
public class VulnObject
3939
{
40+
private static final String CVSS_V4 = "CVSS_V4";
41+
private static final String CVSS_V3 = "CVSS_V3";
42+
private static final String CVSS_V2 = "CVSS_V2";
43+
4044
private List<AffectObject> affected;
4145

4246
private String id;
@@ -127,18 +131,35 @@ public String getMainURL()
127131
}
128132

129133
/**
130-
* @return the CVSS V3 {@link #getSeverity()}
134+
* @return the CVSS {@link #getSeverity()}
131135
*/
132-
public String getSeverityCCSV3()
136+
public String getSeverityCVSS()
133137
{
134138
if (this.severity == null) {
135139
return "";
136140
}
137141
return this.severity.stream()
138-
.filter(s -> Objects.equals("CVSS_V3", s.getType()))
139-
.map(SeverityObject::getScore)
140-
.findFirst()
141-
.orElse(null);
142+
.filter(s -> s.getType().matches("^(CVSS_V)[2-4]$"))
143+
.max((t1, t2) -> {
144+
int result;
145+
if (t1.getType().equals(t2.getType())) {
146+
result = 0;
147+
} else if (CVSS_V4.equals(t1.getType())) {
148+
result = 1;
149+
} else if (CVSS_V4.equals(t2.getType())) {
150+
result = -1;
151+
} else if (CVSS_V3.equals(t1.getType())) {
152+
result = 1;
153+
} else if (CVSS_V3.equals(t2.getType())) {
154+
result = -1;
155+
} else if (CVSS_V2.equals(t1.getType())) {
156+
result = 1;
157+
} else {
158+
result = -1;
159+
}
160+
return result;
161+
})
162+
.map(SeverityObject::getScore).orElse(null);
142163
}
143164

144165
/**

‎xwiki-platform-core/xwiki-platform-extension/xwiki-platform-extension-security/xwiki-platform-extension-security-index/src/test/java/org/xwiki/extension/security/internal/analyzer/osv/OsvResponseAnalyzerTest.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,62 @@ void fixVersion()
124124
extensionSecurityAnalysisResult.getSecurityVulnerabilities().get(0).getFixVersion());
125125
}
126126

127+
@Test
128+
void analyzeCommonsFileuploadOsvResponse()
129+
{
130+
OsvResponse osvResponse = readJson("commons-fileupload-commons-fileupload-1.5.json");
131+
ExtensionSecurityAnalysisResult extensionSecurityAnalysisResult =
132+
this.analyzer.analyzeOsvResponse("commons-fileupload:commons-fileupload", "1.5", osvResponse);
133+
assertEquals(1, extensionSecurityAnalysisResult.getSecurityVulnerabilities().size());
134+
assertEquals(8.7, extensionSecurityAnalysisResult.getMaxCVSS());
135+
}
136+
137+
@Test
138+
void analyzeOsvResponseCVSSV2()
139+
{
140+
OsvResponse osvResponse = readJson("osvResponseCVSSV2.json");
141+
142+
ExtensionSecurityAnalysisResult expected = new ExtensionSecurityAnalysisResult();
143+
SecurityVulnerabilityDescriptor securityVulnerabilityDescriptor = new SecurityVulnerabilityDescriptor();
144+
securityVulnerabilityDescriptor.setId("CVE-1");
145+
securityVulnerabilityDescriptor.setAliases(Set.of("A1", "VULN_ID"));
146+
securityVulnerabilityDescriptor.setURL("https://main.ref/");
147+
securityVulnerabilityDescriptor.setScore(7.5);
148+
securityVulnerabilityDescriptor.setFixVersion(new DefaultVersion("15.7"));
149+
expected.setResults(List.of(securityVulnerabilityDescriptor));
150+
151+
assertEquals(expected,
152+
this.analyzer.analyzeOsvResponse("org.test:my-ext", "7.5", osvResponse));
153+
}
154+
155+
@Test
156+
void analyzeOsvResponseMultipleSeverities()
157+
{
158+
OsvResponse osvResponse = readJson("osvResponseMultipleSeverities.json");
159+
160+
ExtensionSecurityAnalysisResult expected = new ExtensionSecurityAnalysisResult();
161+
SecurityVulnerabilityDescriptor securityVulnerabilityDescriptor = new SecurityVulnerabilityDescriptor();
162+
securityVulnerabilityDescriptor.setId("CVE-1");
163+
securityVulnerabilityDescriptor.setAliases(Set.of("A1", "VULN_ID"));
164+
securityVulnerabilityDescriptor.setURL("https://main.ref/");
165+
securityVulnerabilityDescriptor.setScore(8.7);
166+
securityVulnerabilityDescriptor.setFixVersion(new DefaultVersion("15.7"));
167+
expected.setResults(List.of(securityVulnerabilityDescriptor));
168+
169+
assertEquals(expected,
170+
this.analyzer.analyzeOsvResponse("org.test:my-ext", "7.5", osvResponse));
171+
}
172+
173+
@Test
174+
void analyzeCommonsHttpclientOsvResponse()
175+
{
176+
OsvResponse osvResponse = readJson("commons-httpclient-commons-httpclient-3.1.json");
177+
ExtensionSecurityAnalysisResult extensionSecurityAnalysisResult =
178+
this.analyzer.analyzeOsvResponse("commons-httpclient:commons-httpclient", "3.1", osvResponse);
179+
assertEquals(1, extensionSecurityAnalysisResult.getSecurityVulnerabilities().size());
180+
assertEquals(0, extensionSecurityAnalysisResult.getMaxCVSS());
181+
}
182+
127183
private OsvResponse readJson(String name)
128184
{
129185
InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(name);

0 commit comments

Comments
 (0)