Merge remote-tracking branch 'refs/remotes/origin/master'

Evolveum · Apr 17, 2019 · a81ff07 · a81ff07
2 parents 21e4f8e + e83332d
commit a81ff07
Show file tree

Hide file tree

Showing 12 changed files with 268 additions and 54 deletions.
diff --git a/infra/common/src/main/java/com/evolveum/midpoint/common/validator/LegacyValidator.java b/infra/common/src/main/java/com/evolveum/midpoint/common/validator/LegacyValidator.java
@@ -190,6 +190,11 @@ public void validate(InputStream inputStream, OperationResult validatorResult, S
 			stream = xmlInputFactory.createXMLStreamReader(inputStream);
 
 			int eventType = stream.nextTag();
+			if (eventType == XMLStreamConstants.DTD || eventType == XMLStreamConstants.ENTITY_DECLARATION 
+					|| eventType == XMLStreamConstants.ENTITY_REFERENCE  || eventType == XMLStreamConstants.NOTATION_DECLARATION) {
+				// We do not want those, e.g. we want to void XXE vulnerabilities. Make this check explicit.
+				throw new SystemException("Use of "+eventType+" in XML is prohibited");
+			}
 			if (eventType == XMLStreamConstants.START_ELEMENT) {
 				if (!QNameUtil.match(stream.getName(), SchemaConstants.C_OBJECTS)) {
 					// This has to be an import file with a single objects. Try

diff --git a/...rism-impl/src/main/java/com/evolveum/midpoint/prism/impl/lex/dom/DomLexicalProcessor.java b/...rism-impl/src/main/java/com/evolveum/midpoint/prism/impl/lex/dom/DomLexicalProcessor.java
@@ -105,6 +105,14 @@ public List<RootXNodeImpl> readObjects(@NotNull ParserSource source, @NotNull Pa
 			}
 		}
 	}
+
+	private XMLInputFactory getXMLInputFactory() {
+		XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+		xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); 
+		xmlInputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
+		// TODO: cache? static? prism context?
+		return xmlInputFactory;
+	}
 
 	// code taken from Validator class
 	@Override
@@ -113,7 +121,7 @@ public void readObjectsIteratively(@NotNull ParserSource source, @NotNull Parsin
 		InputStream is = source.getInputStream();
 		XMLStreamReader stream = null;
 		try {
-			stream = XMLInputFactory.newInstance().createXMLStreamReader(is);
+			stream = getXMLInputFactory().createXMLStreamReader(is);
 
 			int eventType = stream.nextTag();
 			if (eventType != XMLStreamConstants.START_ELEMENT) {

diff --git a/...rism-impl/src/main/java/com/evolveum/midpoint/prism/impl/schema/SchemaToDomProcessor.java b/...rism-impl/src/main/java/com/evolveum/midpoint/prism/impl/schema/SchemaToDomProcessor.java
@@ -597,10 +597,17 @@ private void init() throws ParserConfigurationException {
 			LOGGER.trace("Using namespace prefix mapper to serialize schema:\n{}",DebugUtil.dump(namespacePrefixMapper));
 		}
 
-		DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
-		dbf.setNamespaceAware(true);
-		dbf.setValidating(false);
-		DocumentBuilder db = dbf.newDocumentBuilder();
+		DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+		documentBuilderFactory.setNamespaceAware(true);
+		documentBuilderFactory.setValidating(false);
+		// XXE
+		documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+		documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+		documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+		documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+		documentBuilderFactory.setXIncludeAware(false);
+		documentBuilderFactory.setExpandEntityReferences(false);
+		DocumentBuilder db = documentBuilderFactory.newDocumentBuilder();
 
 		document = db.newDocument();
 		Element root = createElement(new QName(W3C_XML_SCHEMA_NS_URI, "schema"));

diff --git a/infra/prism-impl/src/test/java/com/evolveum/midpoint/prism/TestPrismParsingXml.java b/infra/prism-impl/src/test/java/com/evolveum/midpoint/prism/TestPrismParsingXml.java
@@ -15,11 +15,19 @@
  */
 package com.evolveum.midpoint.prism;
 
-import static com.evolveum.midpoint.prism.PrismInternalTestUtil.*;
+import static org.testng.AssertJUnit.assertTrue;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.USER_JACK_ADHOC_BASENAME;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.USER_JACK_FILE_BASENAME;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.USER_JACK_OID;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.assertPropertyValue;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.assertUserJack;
+import static com.evolveum.midpoint.prism.PrismInternalTestUtil.constructInitializedPrismContext;
+import static org.testng.AssertJUnit.assertEquals;
 import static org.testng.AssertJUnit.assertNotNull;
 
 import java.io.IOException;
 
+import org.testng.AssertJUnit;
 import org.testng.annotations.Test;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -30,6 +38,8 @@
 
 public class TestPrismParsingXml extends TestPrismParsing {
 
+	public static final String USER_JACK_XXE_BASENAME = "user-jack-xxe";
+
 	@Override
 	protected String getSubdirName() {
 		return "xml";
@@ -88,6 +98,44 @@ public void testPrismParseDomAdhoc() throws Exception {
 
 		assertUserAdhoc(user, true);
 	}
+
+	@Test
+	public void testPrismParseXxe() throws Exception {
+		final String TEST_NAME = "testPrismParseXxe";
+		PrismInternalTestUtil.displayTestTitle(TEST_NAME);
+
+		PrismContext prismContext = constructInitializedPrismContext();
+
+		try {
+			// WHEN
+			prismContext.parseObject(getFile(USER_JACK_XXE_BASENAME));
+
+			AssertJUnit.fail("Unexpected success");
+		} catch (IllegalStateException e) {
+			// THEN
+			System.out.println("Expected exception: "+e);
+			assertTrue("Unexpected exception message: "+e.getMessage(), e.getMessage().contains("DOCTYPE"));
+		}
+
+	}
+
+	@Test
+	public void testPrismParseDomXxe() throws Exception {
+		final String TEST_NAME = "testPrismParseDomXxe";
+		PrismInternalTestUtil.displayTestTitle(TEST_NAME);
+
+		try {
+			// WHEN
+			DOMUtil.parseFile(getFile(USER_JACK_XXE_BASENAME));
+
+			AssertJUnit.fail("Unexpected success");
+		} catch (IllegalStateException e) {
+			// THEN
+			System.out.println("Expected exception: "+e);
+			assertTrue("Unexpected exception message: "+e.getMessage(), e.getMessage().contains("DOCTYPE"));
+		}
+
+	}
 
 	@Override
 	protected void validateXml(String xmlString, PrismContext prismContext) throws SAXException, IOException {

diff --git a/infra/prism-impl/src/test/resources/common/xml/user-jack-xxe.xml b/infra/prism-impl/src/test/resources/common/xml/user-jack-xxe.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE ANY[ <!ENTITY file SYSTEM "file:///etc/hostname"> ]>
+<!--
+  ~ Copyright (c) 2010-2019 Evolveum
+  ~
+  ~ 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.
+  -->
+<user oid="c0c010c0-d34d-b33f-f00d-111111111111" version="42"
+		xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
+		xmlns:xsd='http://www.w3.org/2001/XMLSchema'
+		xmlns='http://midpoint.evolveum.com/xml/ns/test/foo-1.xsd'
+		xmlns:ds="http://www.w3.org/2000/09/xmldsig#"  
+        xmlns:enc="http://www.w3.org/2001/04/xmlenc#"
+		xmlns:a="http://prism.evolveum.com/xml/ns/public/annotation-3"
+		xmlns:t="http://prism.evolveum.com/xml/ns/public/types-3"
+		xmlns:q="http://prism.evolveum.com/xml/ns/public/query-3"
+		xmlns:adhoc="http://midpoint.evolveum.com/xml/ns/test/adhoc-1.xsd"
+		xmlns:ext="http://midpoint.evolveum.com/xml/ns/test/extension">
+    <name>jack</name>
+    <description>&file;</description>
+    <fullName>cpt. Jack Sparrow</fullName>
+    <givenName>Jack</givenName>
+    <familyName>Sparrow</familyName>
+
+</user>
diff --git a/infra/util/src/main/java/com/evolveum/midpoint/util/DOMUtil.java b/infra/util/src/main/java/com/evolveum/midpoint/util/DOMUtil.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 Evolveum
+ * Copyright (c) 2010-2019 Evolveum
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -171,6 +171,13 @@ public class DOMUtil {
 				documentBuilderFactory.setFeature("http://xml.org/sax/features/validation", false);
 				documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
 				documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+				// XXE
+				documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+				documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+				documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+				documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+				documentBuilderFactory.setXIncludeAware(false);
+				documentBuilderFactory.setExpandEntityReferences(false);
 				return documentBuilderFactory.newDocumentBuilder();
 			} catch (ParserConfigurationException e) {
 				throw new RuntimeException(e);

diff --git a/model/model-client/src/main/java/com/evolveum/midpoint/model/client/ModelClientUtil.java b/model/model-client/src/main/java/com/evolveum/midpoint/model/client/ModelClientUtil.java
@@ -510,9 +510,17 @@ public static String toString(PolyStringType poly) {
 
 	static {
 		try {
-			DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-			factory.setNamespaceAware(true);
-			domDocumentBuilder = factory.newDocumentBuilder();
+			DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+			documentBuilderFactory.setNamespaceAware(true);
+			// XXE
+			documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+			documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+			documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+			documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+			documentBuilderFactory.setXIncludeAware(false);
+			documentBuilderFactory.setExpandEntityReferences(false);
+			domDocumentBuilder = documentBuilderFactory.newDocumentBuilder();
+
 		} catch (ParserConfigurationException e) {
 			throw new IllegalStateException("Error creating XML document " + e.getMessage());
 		}

diff --git a/...report-impl/src/test/java/com/evolveum/midpoint/report/AbstractReportIntegrationTest.java b/...report-impl/src/test/java/com/evolveum/midpoint/report/AbstractReportIntegrationTest.java
@@ -76,7 +76,7 @@ public class AbstractReportIntegrationTest extends AbstractModelIntegrationTest
 	protected final static File USER_ADMINISTRATOR_FILE = new File(TEST_DIR_COMMON, "user-administrator.xml");
 
 	@Autowired protected ReportManager reportManager;
-	@Autowired 
+	@Autowired
 	@Qualifier("reportJasperCreateTaskHandler")
 	protected ReportJasperCreateTaskHandler reportTaskHandler;
 

diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/DataSourceFactory.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/DataSourceFactory.java
@@ -39,6 +39,7 @@ public class DataSourceFactory {
 
     private SqlRepositoryConfiguration configuration;
 
+    private DataSource internalDataSource;
     private DataSource dataSource;
 
     public SqlRepositoryConfiguration getConfiguration() {
@@ -57,24 +58,27 @@ public DataSource createDataSource() throws RepositoryServiceFactoryException {
 
         try {
             if (StringUtils.isNotEmpty(configuration.getDataSource())) {
-                LOGGER.info("JDNI datasource present in configuration, looking for '{}'.",
-                        new Object[]{configuration.getDataSource()});
-                return createJNDIDataSource();
+                LOGGER.info("JNDI datasource present in configuration, looking for '{}'.", configuration.getDataSource());
+                dataSource = createJNDIDataSource();
+            } else {
+                LOGGER.info("Constructing default datasource with connection pooling; JDBC URL: {}", configuration.getJdbcUrl());
+                internalDataSource = createDataSourceInternal();
+                dataSource = internalDataSource;
             }
-
-            LOGGER.info("Constructing default datasource with connection pooling; JDBC URL: {}", configuration.getJdbcUrl());
-            dataSource = createDataSourceInternal();
             return dataSource;
         } catch (Exception ex) {
             throw new RepositoryServiceFactoryException("Couldn't initialize datasource, reason: " + ex.getMessage(), ex);
         }
     }
 
+    public DataSource getDataSource() {
+        return dataSource;
+    }
+
     private DataSource createJNDIDataSource() throws IllegalArgumentException, NamingException {
         JndiObjectFactoryBean factory = new JndiObjectFactoryBean();
         factory.setJndiName(configuration.getDataSource());
         factory.afterPropertiesSet();
-
         return (DataSource) factory.getObject();
     }
 
@@ -135,8 +139,8 @@ private DataSource createDataSourceInternal() {
 
     @PreDestroy
     public void destroy() throws IOException {
-        if (dataSource instanceof Closeable) {
-            ((Closeable) dataSource).close();
+        if (internalDataSource instanceof Closeable) {
+            ((Closeable) internalDataSource).close();
         }
     }
 }