GEOT-5514: Disabled DTD's in gt-xml to protect against XML External E…

…ntity Injection (XEE) attacks Also added PowerMockito in order to add tests for DocumentFactory.java and protect against regression.

modules/library/xml/pom.xml

@@ -135,6 +135,18 @@
         <version>1.9.5</version>
         <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <version>1.5.6</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito</artifactId>
+      <version>1.5.6</version>
+      <scope>test</scope>
+    </dependency>
 
   </dependencies>
 

modules/library/xml/src/main/java/org/geotools/xml/DocumentFactory.java

@@ -141,16 +141,18 @@ public static Object getInstance(InputStream is, Map hints, Level level) throws
     }
 
     /*
-     * convinience method to create an instance of a SAXParser if it is null.
+     * Convenience method to create an instance of a SAXParser if it is null.
      */
     private static SAXParser getParser() throws SAXException {
         SAXParserFactory spf = SAXParserFactory.newInstance();
         spf.setNamespaceAware(true);
         spf.setValidating(false);
 
         try {
-            // spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-            // spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            // The following configuration prevents XML External Entity Injection (XXE) attacks
+            // See for more information: https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing
+            spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
 
             SAXParser sp = spf.newSAXParser();
             return sp;

modules/library/xml/src/test/java/org/geotools/xml/DocumentFactoryTest.java

@@ -0,0 +1,158 @@
+package org.geotools.xml;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.xml.sax.SAXException;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.logging.Level;
+
+import static org.mockito.Mockito.*;
+
+/**
+ * GeoTools - The Open Source Java GIS Toolkit
+ * http://geotools.org
+ * <p>
+ * (C) 2016, Open Source Geospatial Foundation (OSGeo)
+ * <p>
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ * <p>
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * @author Aaron Waddell
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ DocumentFactory.class, SAXParserFactory.class })
+public class DocumentFactoryTest {
+
+        private final String EXTERNAL_GENERAL_ENTITIES_FEATURE =
+                "http://xml.org/sax/features/external-general-entities";
+
+        private final String EXTERNAL_PARAMETER_ENTITIES_FEATURE =
+                "http://xml.org/sax/features/external-parameter-entities";
+
+        private URI uri;
+
+        private Map hints;
+
+        @Mock
+        private SAXParserFactory mockSaxParserFactory;
+
+        @Mock
+        private SAXParser mockSaxParser;
+
+        @Mock
+        InputStream inputStream;
+
+        @Before
+        public void before() throws Exception {
+                uri = new URI("http://geotools.org");
+                hints = new HashMap<String, String>();
+
+                MockitoAnnotations.initMocks(this);
+
+                PowerMockito.mockStatic(SAXParserFactory.class);
+                PowerMockito.when(SAXParserFactory.newInstance()).thenReturn(mockSaxParserFactory);
+
+                when(mockSaxParserFactory.newSAXParser()).thenReturn(mockSaxParser);
+
+                Answer<Void> startDocumentAnswer = new Answer<Void>() {
+                        @Override
+                        public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
+                                XMLSAXHandler xmlSaxHandler = (XMLSAXHandler) invocationOnMock
+                                        .getArguments()[1];
+                                xmlSaxHandler.startDocument();
+                                return null;
+                        }
+                };
+                doAnswer(startDocumentAnswer).when(mockSaxParser)
+                        .parse(anyString(), any(XMLSAXHandler.class));
+                doAnswer(startDocumentAnswer).when(mockSaxParser)
+                        .parse(any(InputStream.class), any(XMLSAXHandler.class));
+        }
+
+        @Test
+        public void testGetInstanceFromURINoSpecifiedLevel() throws Exception {
+                DocumentFactory.getInstance(uri, hints);
+
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_GENERAL_ENTITIES_FEATURE, false);
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+        }
+
+        @Test
+        public void testGetInstanceFromURIWithSpecifiedLevel() throws Exception {
+                DocumentFactory.getInstance(uri, hints, Level.WARNING);
+
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_GENERAL_ENTITIES_FEATURE, false);
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+        }
+
+        @Test(expected = SAXException.class)
+        public void testGetInstanceFromURIWithSpecifiedLevelParseFailureIOException()
+                throws Exception {
+                doThrow(new IOException("Failure")).when(mockSaxParser)
+                        .parse(anyString(), any(XMLSAXHandler.class));
+                DocumentFactory.getInstance(uri, hints, Level.WARNING);
+        }
+
+        @Test(expected = SAXException.class)
+        public void testGetInstanceFromURIWithSpecifiedLevelParseFailureSAXException()
+                throws Exception {
+                doThrow(new SAXException("Failure")).when(mockSaxParser)
+                        .parse(anyString(), any(XMLSAXHandler.class));
+                DocumentFactory.getInstance(uri, hints, Level.WARNING);
+        }
+
+        @Test(expected = SAXException.class)
+        public void testGetInstanceFromURIWithSpecifiedLevelNewSAXParserFailure() throws Exception {
+                when(mockSaxParserFactory.newSAXParser())
+                        .thenThrow(new ParserConfigurationException("Failure"));
+                DocumentFactory.getInstance(uri, hints, Level.WARNING);
+        }
+
+        @Test
+        public void testGetInstanceFromInputStreamWithSpecifiedLevel() throws Exception {
+                DocumentFactory.getInstance(inputStream, hints, Level.WARNING);
+
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_GENERAL_ENTITIES_FEATURE, false);
+                verify(mockSaxParserFactory).setFeature(EXTERNAL_PARAMETER_ENTITIES_FEATURE, false);
+        }
+
+        @Test(expected = SAXException.class)
+        public void testGetInstanceFromInputStreamWithSpecifiedLevelParseFailureIOException()
+                throws Exception {
+                doThrow(new IOException("Parse failure")).when(mockSaxParser)
+                        .parse(any(InputStream.class), any(XMLSAXHandler.class));
+                DocumentFactory.getInstance(inputStream, hints, Level.WARNING);
+        }
+
+        @Test(expected = SAXException.class)
+        public void testGetInstanceFromInputStreamWithSpecifiedLevelParseFailureSAXException()
+                throws Exception {
+                doThrow(new SAXException("Parse failure")).when(mockSaxParser)
+                        .parse(any(InputStream.class), any(XMLSAXHandler.class));
+                DocumentFactory.getInstance(inputStream, hints, Level.WARNING);
+        }
+
+}

pom.xml

@@ -1083,6 +1083,18 @@
         <artifactId>mockito-core</artifactId>
         <version>1.8.5</version>
       </dependency>
+      <dependency>
+        <groupId>org.powermock</groupId>
+        <artifactId>powermock-module-junit4</artifactId>
+        <version>1.4.9</version>
+        <scope>test</scope>
+      </dependency>
+      <dependency>
+        <groupId>org.powermock</groupId>
+        <artifactId>powermock-api-mockito</artifactId>
+        <version>1.4.9</version>
+        <scope>test</scope>
+      </dependency>
 
      <!-- ArcSDE -->
       <dependency>