Skip to content

Commit c9c3ceb

Browse files
davewichersdavewichers
authored
Suggested pom.xml updates and XmlEsapiPropertyLoader.java improvements (#612)
Close issue #614 Pom changes: * Upgrade a few libraries, add some used but undeclared libraries, and delete a bunch of declared/unused libraries. * Upgrade a bunch of the plugins. * Suggested updates to XmlEsapiPropertyLoader.java to eliminate insider XXE risk, and other minor suggested cleanup. * A few more tweaks to use autoclosables to ensure streams get closed. * Remove debug lines.
1 parent 504197b commit c9c3ceb

File tree

2 files changed

+78
-80
lines changed

2 files changed

+78
-80
lines changed

‎pom.xml‎

Lines changed: 60 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,12 @@
132132

133133
<properties>
134134
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
135-
<version.jmh>1.23</version.jmh>
135+
<version.jmh>1.28</version.jmh>
136+
<!-- Note: powermock v2.0.8 doesn't exist. v2.0.9+ requires mockito-core v3+, which requires Java 8 -->
136137
<version.powermock>2.0.7</version.powermock>
137-
<version.spotbugs>4.2.0</version.spotbugs>
138-
139-
<!-- Upgrading to 3.0.0-M3+ causes this test case error:
140-
org.owasp.esapi.reference.DefaultValidatorInputStringAPITest.getValidInputNullAllowedPassthrough Time elapsed: 2.057 s <<< ERROR!
141-
java.lang.OutOfMemoryError: PermGen space
142-
when running tests with Java 7 on Mac OS X. No problems observed on Linux.
143-
-->
144-
<version.surefire>3.0.0-M2</version.surefire>
138+
<version.spotbugs>4.2.2</version.spotbugs>
139+
<version.spotbugs.maven>4.2.2</version.spotbugs.maven>
140+
<version.surefire>3.0.0-M5</version.surefire>
145141
</properties>
146142

147143
<dependencies>
@@ -235,8 +231,8 @@
235231
</dependency>
236232
<dependency>
237233
<groupId>org.apache-extras.beanshell</groupId>
238-
<artifactId>bsh</artifactId>
239-
<version>2.0b6</version>
234+
<artifactId>bsh</artifactId>
235+
<version>2.0b6</version>
240236
</dependency>
241237
<dependency>
242238
<groupId>org.owasp.antisamy</groupId>
@@ -248,68 +244,38 @@
248244
<artifactId>slf4j-api</artifactId>
249245
<version>1.7.30</version>
250246
</dependency>
251-
252-
<!--
253-
FORCE SPECIFIC VERSIONS OF TRANSITIVE DEPENDENCIES EXCLUDED ABOVE.
254-
This is to force patched versions of these libraries with known CVEs against them.
255-
-->
256-
<dependency>
257-
<groupId>commons-io</groupId>
258-
<artifactId>commons-io</artifactId>
259-
<!-- Note: commons-io:2.7+ require Java 8, so can't upgrade past 2.6 -->
260-
<version>2.6</version>
261-
</dependency>
262-
<dependency>
263-
<groupId>org.apache.xmlgraphics</groupId>
264-
<artifactId>batik-css</artifactId>
265-
<version>1.14</version>
266-
<exclusions>
267-
<exclusion>
268-
<groupId>commons-io</groupId>
269-
<artifactId>commons-io</artifactId>
270-
</exclusion>
271-
<exclusion>
272-
<groupId>commons-logging</groupId>
273-
<artifactId>commons-logging</artifactId>
274-
</exclusion>
275-
</exclusions>
276-
</dependency>
277-
<dependency>
278-
<groupId>xalan</groupId>
279-
<artifactId>xalan</artifactId>
280-
<version>2.7.2</version>
281-
<exclusions>
282-
<exclusion>
283-
<groupId>xml-apis</groupId>
284-
<artifactId>xml-apis</artifactId>
285-
</exclusion>
286-
</exclusions>
287-
</dependency>
288247
<dependency>
289248
<groupId>xml-apis</groupId>
290249
<artifactId>xml-apis</artifactId>
291250
<version>1.4.01</version>
292251
</dependency>
293252

253+
<!--
254+
FORCE SPECIFIC VERSIONS OF TRANSITIVE DEPENDENCIES EXCLUDED ABOVE.
255+
This is to force patched versions of these libraries with known CVEs against them.
256+
-->
257+
258+
<!-- No forced upgrades required currently -->
259+
294260
<!-- SpotBugs dependencies -->
295261
<dependency>
296262
<groupId>com.github.spotbugs</groupId>
297263
<artifactId>spotbugs-annotations</artifactId>
298264
<version>${version.spotbugs}</version>
299265
<optional>true</optional>
300266
</dependency>
301-
<dependency>
302-
<groupId>net.jcip</groupId>
303-
<artifactId>jcip-annotations</artifactId>
304-
<version>1.0</version>
305-
<optional>true</optional>
306-
</dependency>
307267

308268
<!-- Dependencies which are ONLY used for JUnit tests -->
269+
<dependency>
270+
<groupId>commons-codec</groupId>
271+
<artifactId>commons-codec</artifactId>
272+
<version>1.15</version>
273+
<scope>test</scope>
274+
</dependency>
309275
<dependency>
310276
<groupId>junit</groupId>
311277
<artifactId>junit</artifactId>
312-
<version>4.13.1</version>
278+
<version>4.13.2</version>
313279
<scope>test</scope>
314280
</dependency>
315281
<dependency>
@@ -318,6 +284,12 @@
318284
<version>1.68</version>
319285
<scope>test</scope>
320286
</dependency>
287+
<dependency>
288+
<groupId>org.hamcrest</groupId>
289+
<artifactId>hamcrest-core</artifactId>
290+
<version>1.3</version>
291+
<scope>test</scope>
292+
</dependency>
321293
<!-- https://mvnrepository.com/artifact/org.powermock/powermock-api-mockito -->
322294
<dependency>
323295
<groupId>org.powermock</groupId>
@@ -351,6 +323,19 @@
351323
<version>2.28.2</version>
352324
<scope>test</scope>
353325
</dependency>
326+
<dependency>
327+
<groupId>org.powermock</groupId>
328+
<artifactId>powermock-core</artifactId>
329+
<version>${version.powermock}</version>
330+
<scope>test</scope>
331+
<exclusions>
332+
<exclusion>
333+
<!-- We exclude this here, because we import the version we need above, and this imports a newer version. -->
334+
<groupId>org.javassist</groupId>
335+
<artifactId>javassist</artifactId>
336+
</exclusion>
337+
</exclusions>
338+
</dependency>
354339
<dependency>
355340
<groupId>org.powermock</groupId>
356341
<artifactId>powermock-module-junit4</artifactId>
@@ -389,12 +374,6 @@
389374
<version>${version.jmh}</version>
390375
<scope>test</scope>
391376
</dependency>
392-
<dependency>
393-
<groupId>org.openjdk.jmh</groupId>
394-
<artifactId>jmh-generator-annprocess</artifactId>
395-
<version>${version.jmh}</version>
396-
<scope>test</scope>
397-
</dependency>
398377
</dependencies>
399378

400379
<build>
@@ -419,6 +398,21 @@
419398
</pluginManagement>
420399

421400
<plugins>
401+
<plugin>
402+
<groupId>com.github.spotbugs</groupId>
403+
<artifactId>spotbugs-maven-plugin</artifactId>
404+
<version>${version.spotbugs.maven}</version>
405+
<dependencies>
406+
<!-- Overwrite dependency on SpotBugs if you want to specify the version of SpotBugs.
407+
SpotBugs itself is frequently several versions ahead of the spotbugs-maven-plugin -->
408+
<dependency>
409+
<groupId>com.github.spotbugs</groupId>
410+
<artifactId>spotbugs</artifactId>
411+
<version>${version.spotbugs}</version>
412+
</dependency>
413+
</dependencies>
414+
</plugin>
415+
422416
<plugin>
423417
<groupId>net.sourceforge.maven-taglib</groupId>
424418
<artifactId>maven-taglib-plugin</artifactId>
@@ -622,19 +616,19 @@
622616
<plugin>
623617
<groupId>org.apache.maven.plugins</groupId>
624618
<artifactId>maven-pmd-plugin</artifactId>
625-
<version>3.13.0</version>
619+
<version>3.14.0</version>
626620
</plugin>
627621

628622
<plugin>
629623
<groupId>org.apache.maven.plugins</groupId>
630624
<artifactId>maven-project-info-reports-plugin</artifactId>
631-
<version>3.1.0</version>
625+
<version>3.1.1</version>
632626
</plugin>
633627

634628
<plugin>
635629
<groupId>org.apache.maven.plugins</groupId>
636630
<artifactId>maven-resources-plugin</artifactId>
637-
<version>3.1.0</version>
631+
<version>3.2.0</version>
638632
</plugin>
639633

640634
<plugin>
@@ -689,7 +683,7 @@
689683
<plugin>
690684
<groupId>org.codehaus.mojo</groupId>
691685
<artifactId>versions-maven-plugin</artifactId>
692-
<version>2.7</version>
686+
<version>2.8.1</version>
693687
</plugin>
694688

695689
<plugin>
@@ -701,7 +695,7 @@
701695
<plugin>
702696
<groupId>org.owasp</groupId>
703697
<artifactId>dependency-check-maven</artifactId>
704-
<version>5.3.2</version>
698+
<version>6.1.3</version>
705699
<configuration>
706700
<failBuildOnCVSS>5.0</failBuildOnCVSS>
707701
<suppressionFiles>./suppressions.xml</suppressionFiles>
@@ -829,7 +823,6 @@
829823
<plugin>
830824
<groupId>com.github.spotbugs</groupId>
831825
<artifactId>spotbugs-maven-plugin</artifactId>
832-
<version>${version.spotbugs}</version>
833826
<configuration>
834827
<plugins>
835828
<plugin>

‎src/main/java/org/owasp/esapi/configuration/XmlEsapiPropertyLoader.java‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
import org.w3c.dom.Element;
77
import org.w3c.dom.Node;
88
import org.w3c.dom.NodeList;
9+
import org.xml.sax.SAXException;
910

1011
import javax.xml.XMLConstants;
1112
import javax.xml.parsers.DocumentBuilder;
1213
import javax.xml.parsers.DocumentBuilderFactory;
14+
import javax.xml.parsers.ParserConfigurationException;
1315
import javax.xml.transform.stream.StreamSource;
1416
import javax.xml.validation.Schema;
1517
import javax.xml.validation.SchemaFactory;
@@ -99,12 +101,14 @@ public String getStringProp(String propertyName) throws ConfigurationException {
99101
/**
100102
* Methods loads configuration from .xml file.
101103
* @param file
104+
* @throws ConfigurationException if there is a problem loading the specified configuration file.
102105
*/
103106
protected void loadPropertiesFromFile(File file) throws ConfigurationException {
104-
try {
105-
validateAgainstXSD(new FileInputStream(file));
107+
try ( InputStream configFile = new FileInputStream(file); ) {
108+
validateAgainstXSD(configFile);
106109

107-
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
110+
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance("org.apache.xerces.jaxp.DocumentBuilderFactoryImpl", null);
111+
dbFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
108112
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
109113
Document doc = dBuilder.parse(file);
110114
doc.getDocumentElement().normalize();
@@ -119,19 +123,20 @@ protected void loadPropertiesFromFile(File file) throws ConfigurationException {
119123
properties.put(propertyKey, propertyValue);
120124
}
121125
}
122-
} catch (Exception e) {
123-
logSpecial("XML config file " + filename + " has invalid schema", e);
124-
throw new ConfigurationException("Configuration file : " + filename + " has invalid schema." + e.getMessage(), e);
126+
} catch (IOException | SAXException | ParserConfigurationException e) {
127+
logSpecial("XML config file " + filename + " doesn't exist or has invalid schema", e);
128+
throw new ConfigurationException("Configuration file : " + filename + " has invalid schema."
129+
+ e.getMessage(), e);
125130
}
126131
}
127132

128-
private void validateAgainstXSD(InputStream xml) throws Exception {
129-
InputStream xsd = getClass().getResourceAsStream("/ESAPI-properties.xsd");
130-
SchemaFactory factory =
131-
SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
132-
Schema schema = factory.newSchema(new StreamSource(xsd));
133-
Validator validator = schema.newValidator();
134-
validator.validate(new StreamSource(xml));
133+
private void validateAgainstXSD(InputStream xml) throws IOException, SAXException {
134+
try ( InputStream xsd = getClass().getResourceAsStream("/ESAPI-properties.xsd"); ) {
135+
SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
136+
Schema schema = factory.newSchema(new StreamSource(xsd));
137+
Validator validator = schema.newValidator();
138+
validator.validate(new StreamSource(xml));
139+
}
135140
}
136141

137142
}

0 commit comments

Comments
 (0)