Merge pull request #25164 from vespa-engine/mortent/xxe

Add more xxe preventions

3 files changed, 49 insertions, 18 deletions

diff --git a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java
index dc5ae998586..2cc5e1ac4e2 100644
--- a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java
+++ b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java
@@ -35,12 +35,7 @@ public class Xml {
     private static final Logger log = Logger.getLogger(Xml.class.getPackage().toString());
 
     // Access to this needs to be synchronized (as it is in getDocumentBuilder() below)
-    private static final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-
-    static {
-        factory.setNamespaceAware(true);
-        factory.setXIncludeAware(false);
-    }
+    private static final DocumentBuilderFactory factory = createDocumentBuilderFactory();
 
     public static Document getDocument(Reader reader) {
         Document doc;
@@ -52,6 +47,23 @@ public class Xml {
         return doc;
     }
 
+    private static DocumentBuilderFactory createDocumentBuilderFactory() {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setXIncludeAware(false);
+
+        try {
+            // XXE prevention
+            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;
+        } catch (ParserConfigurationException e) {
+            log.log(Level.SEVERE, "Could not initialize XML parser", e);
+            throw new RuntimeException(e);
+        }
+    }
+
     /**
      * Creates a new XML document builder.
      *
@@ -67,10 +79,7 @@ public class Xml {
     }
 
     static DocumentBuilder getPreprocessDocumentBuilder() throws ParserConfigurationException {
-        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-        factory.setFeature("http://xml.org/sax/features/external-general-entities", false); // XXE prevention
-        factory.setNamespaceAware(true);
-        factory.setXIncludeAware(false);
+        DocumentBuilderFactory factory = createDocumentBuilderFactory();
         factory.setValidating(false);
         return factory.newDocumentBuilder();
     }
diff --git a/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java b/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java
index 220d6bbb768..9a9a7733c96 100644
--- a/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java
+++ b/config-model/src/main/java/com/yahoo/config/model/builder/xml/XmlHelper.java
@@ -36,13 +36,7 @@ public final class XmlHelper {
 
     private static final String idReference = "idref";
     // Access to this needs to be synchronized (as it is in getDocumentBuilder() below)
-    public static final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-
-    static {
-        XmlHelper.factory.setNamespaceAware(true);
-        // if use jdom and jaxen this will fail badly:
-        XmlHelper.factory.setXIncludeAware(true);
-    }
+    public static final DocumentBuilderFactory factory = createDocumentBuilderFactory();
 
     private XmlHelper() {}
 
@@ -184,4 +178,20 @@ public final class XmlHelper {
 
     }
 
+    private static DocumentBuilderFactory createDocumentBuilderFactory() {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setXIncludeAware(false);
+
+        try {
+            // XXE prevention
+            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;
+        } catch (ParserConfigurationException e) {
+            log.log(Level.SEVERE, "Could not initialize XML parser", e);
+            throw new RuntimeException(e);
+        }
+    }
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java b/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java
index 9780d4b0c17..d50a2e9773d 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/gbdt/XmlHelper.java
@@ -41,12 +41,24 @@ abstract class XmlHelper {
     public static Element parseXmlStream(InputStream in)
             throws ParserConfigurationException, IOException, SAXException
     {
-        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        DocumentBuilderFactory factory = createDocumentBuilderFactory();
         DocumentBuilder builder = factory.newDocumentBuilder();
         Document doc = builder.parse(in);
         return doc.getDocumentElement();
     }
 
+    private static DocumentBuilderFactory createDocumentBuilderFactory() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setXIncludeAware(false);
+
+        // XXE prevention
+        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 String getAttributeText(Node node, String name) {
         Node valueNode = node.getAttributes().getNamedItem(name);
         if (valueNode == null) {