Harden XML parsing via commons-secure-xml

pom.xml

@@ -149,6 +149,11 @@
     </dependencies>
   </dependencyManagement>
   <dependencies>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-secure-xml</artifactId>
+      <version>1.0.0</version>
+    </dependency>
     <dependency>
       <groupId>javax.servlet</groupId>
       <artifactId>servlet-api</artifactId>

src/changes/changes.xml

@@ -49,6 +49,7 @@ The <action> type attribute can be add,update,fix,remove.
     <!-- The release date is the date RC is cut -->
     <release version="1.4.1" date="YYYY-MM-DD" description="This is a maintenance release. Java 8 or later is required.">
       <!-- FIX -->
+      <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Create the DOM and JDOM parsers and the XML transformer through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default.</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory">POM assembly:single does not generate binary convenience files (tar/zip).</action>
       <action type="fix" dev="ggregory" due-to="Dima1224, Gary Gregory">Make dynamicPropertyHandlerMap in ValueUtils thread-safe #251.</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory">Refactor JXPathIntrospector internal static maps to use concurrent classes instead of synchronization.</action>

src/main/java/org/apache/commons/jxpath/XMLDocumentContainer.java

@@ -22,10 +22,10 @@
 
 import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMResult;
 
 import org.apache.commons.jxpath.xml.DocumentContainer;
+import org.apache.commons.xml.secure.SecureTransformerFactory;
 
 /**
  * An XML document container reads and parses XML only when it is accessed. JXPath traverses Containers transparently - you use the same paths to access objects
@@ -85,7 +85,7 @@ public Object getValue() {
             try {
                 if (source != null) {
                     final DOMResult result = new DOMResult();
-                    final Transformer trans = TransformerFactory.newInstance().newTransformer();
+                    final Transformer trans = SecureTransformerFactory.newInstance().newTransformer();
                     trans.transform(source, result);
                     document = result.getNode();
                 } else {

src/main/java/org/apache/commons/jxpath/xml/DOMParser.java

@@ -22,6 +22,7 @@
 import javax.xml.parsers.DocumentBuilderFactory;
 
 import org.apache.commons.jxpath.JXPathException;
+import org.apache.commons.xml.secure.SecureDocumentBuilderFactory;
 
 /**
  * An implementation of the XMLParser interface that produces a DOM Document.
@@ -38,7 +39,7 @@ public DOMParser() {
     @Override
     public Object parseXML(final InputStream stream) {
         try {
-            final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newInstance();
             factory.setValidating(isValidating());
             factory.setNamespaceAware(isNamespaceAware());
             factory.setIgnoringElementContentWhitespace(isIgnoringElementContentWhitespace());

src/main/java/org/apache/commons/jxpath/xml/JDOMParser.java

@@ -19,8 +19,13 @@
 
 import java.io.InputStream;
 
+import javax.xml.parsers.SAXParserFactory;
+
 import org.apache.commons.jxpath.JXPathException;
+import org.apache.commons.xml.secure.SecureSAXParserFactory;
+import org.jdom.JDOMException;
 import org.jdom.input.SAXBuilder;
+import org.xml.sax.XMLReader;
 
 /**
  * An implementation of the XMLParser interface that produces a JDOM Document.
@@ -40,7 +45,19 @@ public Object parseXML(final InputStream stream) {
             throw new JXPathException("JDOM parser configuration error. JDOM does not support the namespaceAware=false setting.");
         }
         try {
-            final SAXBuilder builder = new SAXBuilder();
+            // JDOM builds its reader through JAXP internally; hand it one from the secure factory instead.
+            final SAXBuilder builder = new SAXBuilder() {
+                @Override
+                protected XMLReader createParser() throws JDOMException {
+                    try {
+                        final SAXParserFactory factory = SecureSAXParserFactory.newNSInstance();
+                        factory.setValidating(isValidating());
+                        return factory.newSAXParser().getXMLReader();
+                    } catch (final Exception ex) {
+                        throw new JDOMException("Unable to create a new XML reader", ex);
+                    }
+                }
+            };
             builder.setExpandEntities(isExpandEntityReferences());
             builder.setIgnoringElementContentWhitespace(isIgnoringElementContentWhitespace());
             builder.setValidation(isValidating());