Merge pull request #12 from 3dcitydb/hotfix-prevent-xxe-vulnerabilities

Prevent XXE vulnerabilities

CHANGES.md

@@ -1,6 +1,11 @@
 Change Log
 ==========
 
+### 5.2.1 - tba
+
+#### Fixes
+*  Fixed XXE vulnerabilities when parsing XML files. [#12](https://github.com/3dcitydb/web-feature-service/pull/12/files)
+
 ### 5.2.0 - 2022-05-23
 
 This release is based on the Importer/Exporter version 5.2.0 libraries, and thus incorporates all bug fixes and updates

build.gradle

@@ -39,11 +39,14 @@ repositories {
     maven {
         url 'https://citydb.jfrog.io/artifactory/maven'
     }
+    maven {
+        url 'https://oss.sonatype.org/content/repositories/snapshots/'
+    }
     mavenCentral()
 }
 
 dependencies {
-    implementation 'org.citydb:impexp-core:5.2.0'
+    implementation 'org.citydb:impexp-core:5.2.1-SNAPSHOT'
     implementation 'com.github.seancfoley:ipaddress:5.3.4'
     implementation 'org.glassfish.jersey.containers:jersey-container-servlet:2.35'
     implementation 'org.glassfish.jersey.inject:jersey-hk2:2.35'

src/main/java/vcs/citydb/wfs/WFSService.java

@@ -7,6 +7,7 @@
 import org.citydb.core.util.Util;
 import org.citydb.util.concurrent.SingleWorkerPool;
 import org.citydb.util.log.Logger;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.citygml4j.builder.jaxb.CityGMLBuilder;
 import org.citygml4j.xml.schema.SchemaHandler;
 import org.xml.sax.InputSource;
@@ -90,8 +91,16 @@ public void init() throws ServletException {
 		wfsConfig = registry.lookup(WFSConfig.class);
 
 		exceptionReportHandler = new WFSExceptionReportHandler(cityGMLBuilder);
-		saxParserFactory = SAXParserFactory.newInstance();
-		saxParserFactory.setNamespaceAware(true);
+
+		try {
+			saxParserFactory = SecureXMLProcessors.newSAXParserFactory();
+			saxParserFactory.setNamespaceAware(true);
+		} catch (Throwable e) {
+			String message = "Failed to enable secure processing of XML queries.";
+			log.error(message);
+			log.error(e.getMessage());
+			throw new ServletException(message, e);
+		}
 
 		try {
 			StoredQueryManager storedQueryManager = new StoredQueryManager(cityGMLBuilder, saxParserFactory, getServletContext().getRealPath(Constants.STORED_QUERIES_PATH), wfsConfig);

src/main/java/vcs/citydb/wfs/config/WFSConfigSchemaWriter.java

@@ -1,6 +1,6 @@
 package vcs.citydb.wfs.config;
 
-import org.citydb.config.ConfigUtil;
+import org.citydb.config.util.ConfigConstants;
 
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.SchemaOutputResolver;
@@ -21,7 +21,7 @@ public static void main(String[] args) throws Exception {
 			public Result createOutput(String namespaceUri, String suggestedFileName) throws IOException {
 				File file;
 
-				if (namespaceUri.equals(ConfigUtil.CITYDB_CONFIG_NAMESPACE_URI))
+				if (namespaceUri.equals(ConfigConstants.CITYDB_CONFIG_NAMESPACE_URI))
 					file = new File(Constants.CONFIG_SCHEMA_FILE);
 				else
 					file = new File(Constants.CONFIG_SCHEMA_PATH + "/ows/" + suggestedFileName);

src/main/java/vcs/citydb/wfs/operation/getfeature/citygml/CityGMLWriterBuilder.java

@@ -10,6 +10,7 @@
 import org.citydb.core.operation.exporter.util.InternalConfig;
 import org.citydb.core.operation.exporter.writer.FeatureWriteException;
 import org.citydb.util.log.Logger;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.citygml4j.model.module.Module;
 import org.citygml4j.model.module.ModuleContext;
 import org.citygml4j.model.module.Modules;
@@ -29,7 +30,6 @@
 import javax.xml.datatype.DatatypeConfigurationException;
 import javax.xml.transform.Templates;
 import javax.xml.transform.TransformerConfigurationException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.sax.SAXTransformerFactory;
 import javax.xml.transform.stream.StreamSource;
 import java.io.File;
@@ -140,7 +140,7 @@ public void initializeContext(
 					&& wfsConfig.getPostProcessing().getXSLTransformation().isSetStylesheets()) {
 				try {
 					List<String> stylesheets = wfsConfig.getPostProcessing().getXSLTransformation().getStylesheets();
-					SAXTransformerFactory factory = (SAXTransformerFactory) TransformerFactory.newInstance();
+					SAXTransformerFactory factory = (SAXTransformerFactory) SecureXMLProcessors.newTransformerFactory();
 					Templates[] templates = new Templates[stylesheets.size()];
 
 					for (int i = 0; i < stylesheets.size(); i++) {

src/main/java/vcs/citydb/wfs/operation/getpropertyvalue/GetPropertyValueResponseBuilder.java

@@ -8,6 +8,7 @@
 import org.citydb.core.database.schema.mapping.FeatureType;
 import org.citydb.core.operation.exporter.writer.FeatureWriteException;
 import org.citydb.util.log.Logger;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.citygml4j.model.module.Module;
 import org.citygml4j.model.module.ModuleContext;
 import org.citygml4j.model.module.Modules;
@@ -25,7 +26,6 @@
 import javax.xml.datatype.DatatypeConfigurationException;
 import javax.xml.transform.Templates;
 import javax.xml.transform.TransformerConfigurationException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.sax.SAXTransformerFactory;
 import javax.xml.transform.stream.StreamSource;
 import java.io.File;
@@ -124,7 +124,7 @@ public void initializeContext(GetPropertyValueType wfsRequest,
 					&& wfsConfig.getPostProcessing().getXSLTransformation().isSetStylesheets()) {
 				try {
 					List<String> stylesheets = wfsConfig.getPostProcessing().getXSLTransformation().getStylesheets();
-					SAXTransformerFactory factory = (SAXTransformerFactory) TransformerFactory.newInstance();
+					SAXTransformerFactory factory = (SAXTransformerFactory) SecureXMLProcessors.newTransformerFactory();
 					Templates[] templates = new Templates[stylesheets.size()];
 
 					for (int i = 0; i < stylesheets.size(); i++) {

src/main/java/vcs/citydb/wfs/operation/storedquery/StoredQueryManager.java

@@ -4,6 +4,7 @@
 import net.opengis.fes._2.FilterType;
 import net.opengis.fes._2.ResourceIdType;
 import net.opengis.wfs._2.*;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.citygml4j.builder.jaxb.CityGMLBuilder;
 import org.citygml4j.model.module.citygml.CityGMLModule;
 import org.citygml4j.model.module.citygml.CityGMLModuleType;
@@ -27,6 +28,7 @@
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 import javax.xml.stream.XMLOutputFactory;
+import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathExpression;
@@ -35,6 +37,7 @@
 import java.io.BufferedReader;
 import java.io.FileReader;
 import java.io.IOException;
+import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -58,16 +61,16 @@ public class StoredQueryManager {
 	private final WFSConfig wfsConfig;
 	private final MessageDigest md5;
 
-	public StoredQueryManager(CityGMLBuilder cityGMLBuilder, SAXParserFactory saxParserFactory, String path, WFSConfig wfsConfig) throws ParserConfigurationException, SAXException, NoSuchAlgorithmException, IOException {
+	public StoredQueryManager(CityGMLBuilder cityGMLBuilder, SAXParserFactory saxParserFactory, String path, WFSConfig wfsConfig) throws ParserConfigurationException, SAXException, NoSuchAlgorithmException, IOException, TransformerConfigurationException {
 		this.cityGMLBuilder = cityGMLBuilder;
 		this.saxParserFactory = saxParserFactory;
 		this.wfsConfig = wfsConfig;
 
 		md5 = MessageDigest.getInstance("MD5");
 		wfsFactory = new ObjectFactory();
-		transformerFactory = TransformerFactory.newInstance();
+		transformerFactory = SecureXMLProcessors.newTransformerFactory();
 		xmlOutputFactory = XMLOutputFactory.newInstance();
-		documentBuilderFactory = DocumentBuilderFactory.newInstance();
+		documentBuilderFactory = SecureXMLProcessors.newDocumentBuilderFactory();
 		documentBuilderFactory.setNamespaceAware(true);
 
 		storedQueriesPath = Paths.get(path);
@@ -83,9 +86,11 @@ public List<StoredQueryAdapter> listStoredQueries(String handle) throws WFSExcep
 		storedQueries.add(new StoredQueryAdapter(DEFAULT_QUERY.getId()));
 
 		try {
-			for (Path file : Files.newDirectoryStream(storedQueriesPath)) {
-				if (Files.isRegularFile(file))
-					storedQueries.add(new StoredQueryAdapter(file));
+			try (DirectoryStream<Path> stream = Files.newDirectoryStream(storedQueriesPath)) {
+				for (Path file : stream) {
+					if (Files.isRegularFile(file))
+						storedQueries.add(new StoredQueryAdapter(file));
+				}
 			}
 		} catch (IOException e) {
 			throw new WFSException(WFSExceptionCode.OPERATION_PROCESSING_FAILED, "Failed to list stored queries.", handle, e);