prevent XXE vulnerabilities

clausnagel · clausnagel · commit 8ab7fb6ab142 · 2022-06-10T17:03:46.000+02:00
diff --git a/build.gradle b/build.gradle
@@ -35,6 +35,9 @@ subprojects {
         maven {
             url 'https://citydb.jfrog.io/artifactory/maven'
         }
+        maven {
+            url 'https://oss.sonatype.org/content/repositories/snapshots/'
+        }
         mavenCentral()
     }
 
diff --git a/impexp-client-gui/src/main/java/org/citydb/gui/map/geocoder/service/GoogleGeocoder.java b/impexp-client-gui/src/main/java/org/citydb/gui/map/geocoder/service/GoogleGeocoder.java
@@ -32,12 +32,12 @@
 import org.citydb.gui.map.geocoder.GeocoderResult;
 import org.citydb.gui.map.geocoder.Location;
 import org.citydb.gui.map.geocoder.LocationType;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.jdesktop.swingx.mapviewer.GeoPosition;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
-import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.xpath.XPath;
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathFactory;
@@ -84,9 +84,11 @@ private GeocoderResult geocode(String operation, String requestString) throws Ge
         }
 
         try (InputStream stream = new URL(serviceCall).openStream()) {
-            Document response = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(stream);
-            XPath xpath = XPathFactory.newInstance().newXPath();
+            Document response = SecureXMLProcessors.newDocumentBuilderFactory()
+                    .newDocumentBuilder()
+                    .parse(stream);
 
+            XPath xpath = XPathFactory.newInstance().newXPath();
             GeocoderResult geocodingResult = new GeocoderResult();
 
             // check the response status
diff --git a/impexp-config/build.gradle b/impexp-config/build.gradle
@@ -1,3 +1,3 @@
 dependencies {
-    api 'org.citygml4j:citygml4j:2.12.0'
+    api 'org.citygml4j:citygml4j:2.12.1-SNAPSHOT'
 }
diff --git a/impexp-core/src/main/java/org/citydb/core/database/schema/util/SchemaMappingUtil.java b/impexp-core/src/main/java/org/citydb/core/database/schema/util/SchemaMappingUtil.java
@@ -28,15 +28,21 @@
 package org.citydb.core.database.schema.util;
 
 import org.citydb.core.database.schema.mapping.*;
+import org.citydb.util.xml.SecureXMLProcessors;
+import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
 
 import javax.xml.XMLConstants;
 import javax.xml.bind.*;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
 import javax.xml.validation.Schema;
 import javax.xml.validation.SchemaFactory;
 import java.io.*;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 
 public class SchemaMappingUtil {
 	private static SchemaMappingUtil instance;
@@ -53,56 +59,53 @@ public static synchronized SchemaMappingUtil getInstance() throws JAXBException
 		return instance;
 	}	
 
-	private SchemaMapping unmarshal(SchemaMapping schemaMapping, Object input) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		Unmarshaller um = context.createUnmarshaller();
-		um.setAdapter(new AppSchemaAdapter(schemaMapping));
-		um.setAdapter(new ComplexAttributeTypeAdapter(schemaMapping));
-		um.setAdapter(new ComplexTypeAdapter(schemaMapping));
-		um.setAdapter(new FeatureTypeAdapter(schemaMapping));
-		um.setAdapter(new ObjectTypeAdapter(schemaMapping));
-		um.setListener(new UnmarshalListener());
+	private SchemaMapping unmarshal(SchemaMapping schemaMapping, InputSource input) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
+		Unmarshaller unmarshaller = context.createUnmarshaller();
+		unmarshaller.setAdapter(new AppSchemaAdapter(schemaMapping));
+		unmarshaller.setAdapter(new ComplexAttributeTypeAdapter(schemaMapping));
+		unmarshaller.setAdapter(new ComplexTypeAdapter(schemaMapping));
+		unmarshaller.setAdapter(new FeatureTypeAdapter(schemaMapping));
+		unmarshaller.setAdapter(new ObjectTypeAdapter(schemaMapping));
+		unmarshaller.setListener(new UnmarshalListener());
 
 		// validate schema mapping
 		ValidationEvent[] events = new ValidationEvent[1];
-		um.setSchema(readSchema());
-		um.setEventHandler(new ValidationEventHandler() {
-			public boolean handleEvent(ValidationEvent event) {
-				events[0] = event;
-				return false;
-			}
+		unmarshaller.setSchema(readSchema());
+		unmarshaller.setEventHandler(event -> {
+			events[0] = event;
+			return false;
 		});
 
-		// unmarshal schema mapping
-		Object result = null;
-		try {
-			if (input instanceof InputStream)
-				result = um.unmarshal((InputStream) input);
-			else if (input instanceof Reader)
-				result = um.unmarshal((Reader) input);
+		UnmarshallerHandler handler = unmarshaller.getUnmarshallerHandler();
 
-			if (!(result instanceof SchemaMapping))
-				throw new SchemaMappingException("Failed to unmarshal input resource into a schema mapping.");
-
-			return (SchemaMapping)result;
-		} catch (JAXBException e) {
-			if (events[0] != null)
+		try {
+			SAXParserFactory saxParserFactory = SecureXMLProcessors.newSAXParserFactory();
+			saxParserFactory.setNamespaceAware(true);
+			XMLReader reader = saxParserFactory.newSAXParser().getXMLReader();
+			reader.setContentHandler(handler);
+			reader.parse(input);
+		} catch (SAXException | ParserConfigurationException | IOException e) {
+			if (events[0] != null) {
 				throw new SchemaMappingValidationException(events[0].getMessage());
+			} else {
+				throw new SchemaMappingException("Failed to parse the schema mapping.", e);
+			}
+		}
 
-			throw e;
-		} catch (Throwable e) {
-			if (e.getCause() instanceof SchemaMappingException)
-				throw (SchemaMappingException)e.getCause();
+		// unmarshal schema mapping
+		Object result = handler.getResult();
+		if (!(result instanceof SchemaMapping))
+			throw new SchemaMappingException("Failed to unmarshal input resource into a schema mapping.");
 
-			throw e;
-		}		
+		return (SchemaMapping) result;
 	}
 
 	public SchemaMapping unmarshal(SchemaMapping schemaMapping, InputStream inputStream) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		return unmarshal(schemaMapping, (Object) inputStream);
+		return unmarshal(schemaMapping, new InputSource(inputStream));
 	}
 
 	public SchemaMapping unmarshal(InputStream inputStream) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		return unmarshal(null, (Object) inputStream);
+		return unmarshal(null, new InputSource(inputStream));
 	}
 
 	public SchemaMapping unmarshal(SchemaMapping schemaMapping, URL resource) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
@@ -118,7 +121,7 @@ public SchemaMapping unmarshal(URL resource) throws SchemaMappingException, Sche
 	}
 
 	public SchemaMapping unmarshal(SchemaMapping schemaMapping, File file) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		try (InputStream inputStream = new FileInputStream(file)) {
+		try (InputStream inputStream = Files.newInputStream(file.toPath())) {
 			return unmarshal(schemaMapping, inputStream);
 		} catch (IOException e) {
 			throw new JAXBException("Failed to open schema mapping resource from file '" + file.getAbsolutePath() + "'.");
@@ -130,11 +133,7 @@ public SchemaMapping unmarshal(File file) throws SchemaMappingException, SchemaM
 	}
 
 	public SchemaMapping unmarshal(SchemaMapping schemaMapping, String xml) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		try (Reader reader = new StringReader(xml)) {
-			return unmarshal(schemaMapping, reader);
-		} catch (IOException e) {
-			throw new JAXBException("Failed to open schema mapping resource from XML string.");
-		}
+		return unmarshal(schemaMapping, new InputSource(new StringReader(xml)));
 	}
 
 	public SchemaMapping unmarshal(String xml) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
@@ -151,11 +150,9 @@ public void marshal(SchemaMapping schemaMapping, Writer writer, boolean prettyPr
 		// validate schema mapping
 		ValidationEvent[] events = new ValidationEvent[1];
 		m.setSchema(readSchema());
-		m.setEventHandler(new ValidationEventHandler() {
-			public boolean handleEvent(ValidationEvent event) {
-				events[0] = event;
-				return false;
-			}
+		m.setEventHandler(event -> {
+			events[0] = event;
+			return false;
 		});
 
 		try {
@@ -178,7 +175,7 @@ public void marshal(SchemaMapping schemaMapping, Writer writer) throws SchemaMap
 	}
 	
 	public void marshal(SchemaMapping schemaMapping, File file) throws SchemaMappingException, SchemaMappingValidationException, JAXBException {
-		try (Writer writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(file), StandardCharsets.UTF_8))) {
+		try (Writer writer = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(file.toPath()), StandardCharsets.UTF_8))) {
 			marshal(schemaMapping, writer);
 		} catch (UnsupportedEncodingException e) {
 			throw new JAXBException("Failed to marshal schema mapping.", e);
@@ -189,8 +186,9 @@ public void marshal(SchemaMapping schemaMapping, File file) throws SchemaMapping
 
 	private Schema readSchema() throws JAXBException {
 		try {
-			return SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)
-					.newSchema(SchemaMappingUtil.class.getResource("/org/citydb/core/database/schema/3dcitydb-schema.xsd"));
+			SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+			schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+			return schemaFactory.newSchema(SchemaMappingUtil.class.getResource("/org/citydb/core/database/schema/3dcitydb-schema.xsd"));
 		} catch (SAXException e) {
 			throw new JAXBException("Failed to parse the schema mapping XSD schema. " +
 					"Could not find '/org/citydb/core/database/schema/3dcitydb-schema.xsd' on the classpath.", e);
diff --git a/impexp-core/src/main/java/org/citydb/core/operation/exporter/writer/citygml/CityGMLWriterFactory.java b/impexp-core/src/main/java/org/citydb/core/operation/exporter/writer/citygml/CityGMLWriterFactory.java
@@ -41,6 +41,7 @@
 import org.citydb.core.query.Query;
 import org.citydb.core.query.filter.type.FeatureTypeFilter;
 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;
@@ -55,7 +56,6 @@
 import javax.xml.XMLConstants;
 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;
@@ -94,7 +94,7 @@ public CityGMLWriterFactory(Query query, SchemaMapping schemaMapping, Config con
 				log.info("Applying XSL transformations on export data.");
 
 				List<String> stylesheets = cityGMLOptions.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++) {
diff --git a/impexp-core/src/main/java/org/citydb/core/operation/importer/reader/citygml/CityGMLReaderFactory.java b/impexp-core/src/main/java/org/citydb/core/operation/importer/reader/citygml/CityGMLReaderFactory.java
@@ -37,6 +37,7 @@
 import org.citydb.core.operation.importer.reader.FeatureReaderFactory;
 import org.citydb.core.registry.ObjectRegistry;
 import org.citydb.util.log.Logger;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.citygml4j.builder.jaxb.CityGMLBuilderException;
 import org.citygml4j.model.module.Module;
 import org.citygml4j.model.module.Modules;
@@ -49,7 +50,6 @@
 import javax.xml.namespace.QName;
 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;
@@ -101,7 +101,7 @@ public void initializeContext(CityGMLFilter filter, Config config) throws Featur
                 log.info("Applying XSL transformations to CityGML input features.");
 
                 List<String> stylesheets = cityGMLOptions.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++) {
diff --git a/impexp-core/src/main/java/org/citydb/core/operation/validator/reader/citygml/CityGMLValidatorFactory.java b/impexp-core/src/main/java/org/citydb/core/operation/validator/reader/citygml/CityGMLValidatorFactory.java
@@ -48,6 +48,7 @@ public void initializeContext(Config config) throws ValidationException {
         try {
             SchemaHandler schemaHandler = SchemaHandler.newInstance();
             SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+            schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
             Schema schema = schemaFactory.newSchema(schemaHandler.getSchemaSources());
             validator = schema.newValidator();
         } catch (SAXException e) {
diff --git a/impexp-util/src/main/java/org/citydb/util/config/ConfigUtil.java b/impexp-util/src/main/java/org/citydb/util/config/ConfigUtil.java
@@ -29,8 +29,9 @@
 
 import org.citydb.config.ProjectConfig;
 import org.citydb.config.gui.GuiConfig;
-import org.citydb.config.util.ConfigNamespaceFilter;
 import org.citydb.config.project.query.QueryWrapper;
+import org.citydb.config.util.ConfigNamespaceFilter;
+import org.citydb.util.xml.SecureXMLProcessors;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 import org.xml.sax.XMLReader;
@@ -95,19 +96,19 @@ public Object unmarshal(File file) throws JAXBException, IOException {
 	public Object unmarshal(InputStream inputStream) throws JAXBException, IOException {
 		Unmarshaller unmarshaller = createJAXBContext().context.createUnmarshaller();
 		UnmarshallerHandler handler = unmarshaller.getUnmarshallerHandler();
-		SAXParserFactory factory = SAXParserFactory.newInstance();
-		factory.setNamespaceAware(true);
 
 		ConfigNamespaceFilter namespaceFilter;
 		try {
+			SAXParserFactory factory = SecureXMLProcessors.newSAXParserFactory();
+			factory.setNamespaceAware(true);
 			XMLReader reader = factory.newSAXParser().getXMLReader();
 			namespaceFilter = new ConfigNamespaceFilter(reader);	
 			namespaceFilter.setContentHandler(handler);
 			namespaceFilter.parse(new InputSource(inputStream));			
 		} catch (SAXException e) {
 			throw new JAXBException(e.getMessage());
 		} catch (ParserConfigurationException e) {
-			throw new IOException(e.getMessage());
+			throw new IOException("Failed to create secure XML reader.", e);
 		}
 
 		Object result = handler.getResult();
diff --git a/impexp-util/src/main/java/org/citydb/util/xml/SecureXMLProcessors.java b/impexp-util/src/main/java/org/citydb/util/xml/SecureXMLProcessors.java
@@ -0,0 +1,75 @@
+/*
+ * 3D City Database - The Open Source CityGML Database
+ * https://www.3dcitydb.org/
+ *
+ * Copyright 2013 - 2022
+ * Chair of Geoinformatics
+ * Technical University of Munich, Germany
+ * https://www.lrg.tum.de/gis/
+ *
+ * The 3D City Database is jointly developed with the following
+ * cooperation partners:
+ *
+ * Virtual City Systems, Berlin <https://vc.systems/>
+ * M.O.S.S. Computer Grafik Systeme GmbH, Taufkirchen <http://www.moss.de/>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.citydb.util.xml;
+
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerFactory;
+
+public class SecureXMLProcessors {
+
+    private SecureXMLProcessors() {
+    }
+
+    public static SAXParserFactory newSAXParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException {
+        SAXParserFactory factory = SAXParserFactory.newInstance();
+        factory.setXIncludeAware(false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        return factory;
+    }
+
+    public static DocumentBuilderFactory newDocumentBuilderFactory() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setXIncludeAware(false);
+        factory.setExpandEntityReferences(false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        return factory;
+    }
+
+    public static TransformerFactory newTransformerFactory() throws TransformerConfigurationException {
+        TransformerFactory factory = TransformerFactory.newInstance();
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+        return factory;
+    }
+}
diff --git a/impexp-vis-plugin/src/main/java/org/citydb/vis/util/ElevationServiceHandler.java b/impexp-vis-plugin/src/main/java/org/citydb/vis/util/ElevationServiceHandler.java

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,9 @@ subprojects {`
`35`	`35`	`maven {`
`36`	`36`	`url 'https://citydb.jfrog.io/artifactory/maven'`
`37`	`37`	`}`
	`38`	`+ maven {`
	`39`	`+ url 'https://oss.sonatype.org/content/repositories/snapshots/'`
	`40`	`+ }`
`38`	`41`	`mavenCentral()`
`39`	`42`	`}`
`40`	`43`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`	`1`	`dependencies {`
`2`		`- api 'org.citygml4j:citygml4j:2.12.0'`
	`2`	`+ api 'org.citygml4j:citygml4j:2.12.1-SNAPSHOT'`
`3`	`3`	`}`