Use a shared TransformerFactory from SecureXmlUtil (#898)

eclipse · Nov 18, 2021 · f094498 · f094498
1 parent f86bfda
commit f094498
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 17 deletions.
diff --git a/.../src/main/java/org/eclipse/milo/opcua/stack/core/serialization/OpcUaXmlStreamDecoder.java b/.../src/main/java/org/eclipse/milo/opcua/stack/core/serialization/OpcUaXmlStreamDecoder.java
@@ -31,7 +31,6 @@
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 
@@ -59,8 +58,8 @@
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.ULong;
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.UShort;
 import org.eclipse.milo.opcua.stack.core.util.ArrayUtil;
-import org.eclipse.milo.opcua.stack.core.util.DocumentBuilderUtil;
 import org.eclipse.milo.opcua.stack.core.util.Namespaces;
+import org.eclipse.milo.opcua.stack.core.util.SecureXmlUtil;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
@@ -84,7 +83,7 @@ public OpcUaXmlStreamDecoder(SerializationContext context) {
         this.context = context;
 
         try {
-            builder = DocumentBuilderUtil.SHARED_FACTORY.newDocumentBuilder();
+            builder = SecureXmlUtil.SHARED_DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();
         } catch (ParserConfigurationException e) {
             throw new UaRuntimeException(StatusCodes.Bad_InternalError, e);
         }
@@ -1272,7 +1271,7 @@ private static XmlElement nodeToXmlElement(Node node) throws UaSerializationExce
         try {
             StringWriter sw = new StringWriter();
 
-            Transformer transformer = TransformerFactory.newInstance().newTransformer();
+            Transformer transformer = SecureXmlUtil.SHARED_TRANSFORMER_FACTORY.newTransformer();
             transformer.setOutputProperty("omit-xml-declaration", "yes");
             transformer.transform(new DOMSource(node), new StreamResult(sw));
 

diff --git a/.../src/main/java/org/eclipse/milo/opcua/stack/core/serialization/OpcUaXmlStreamEncoder.java b/.../src/main/java/org/eclipse/milo/opcua/stack/core/serialization/OpcUaXmlStreamEncoder.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 the Eclipse Milo Authors
+ * Copyright (c) 2021 the Eclipse Milo Authors
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -35,8 +35,8 @@
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.UInteger;
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.ULong;
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.UShort;
-import org.eclipse.milo.opcua.stack.core.util.DocumentBuilderUtil;
 import org.eclipse.milo.opcua.stack.core.util.Namespaces;
+import org.eclipse.milo.opcua.stack.core.util.SecureXmlUtil;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 
@@ -53,7 +53,7 @@ public OpcUaXmlStreamEncoder(SerializationContext context) {
         this.context = context;
 
         try {
-            builder = DocumentBuilderUtil.SHARED_FACTORY.newDocumentBuilder();
+            builder = SecureXmlUtil.SHARED_DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();
 
             document = builder.newDocument();
             currentNode = document;

diff --git a/.../stack/core/util/DocumentBuilderUtil.java → .../opcua/stack/core/util/SecureXmlUtil.java b/.../stack/core/util/DocumentBuilderUtil.java → .../opcua/stack/core/util/SecureXmlUtil.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 the Eclipse Milo Authors
+ * Copyright (c) 2021 the Eclipse Milo Authors
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -12,39 +12,49 @@
 
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.transform.TransformerFactory;
 
 import org.slf4j.LoggerFactory;
 
-public class DocumentBuilderUtil {
+public class SecureXmlUtil {
 
-    private DocumentBuilderUtil() {}
+    private SecureXmlUtil() {}
 
     /**
      * A shared {@link DocumentBuilderFactory} that has been configured securely to prevent XXE attacks.
      */
-    public static final DocumentBuilderFactory SHARED_FACTORY = DocumentBuilderFactory.newInstance();
+    public static final DocumentBuilderFactory SHARED_DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance();
+
+    /**
+     * A shared {@link TransformerFactory} that has been configured securely to prevent XXE attacks.
+     */
+    public static final TransformerFactory SHARED_TRANSFORMER_FACTORY = TransformerFactory.newInstance();
 
     static {
-        SHARED_FACTORY.setCoalescing(true);
-        SHARED_FACTORY.setNamespaceAware(true);
+        SHARED_DOCUMENT_BUILDER_FACTORY.setCoalescing(true);
+        SHARED_DOCUMENT_BUILDER_FACTORY.setNamespaceAware(true);
 
         // XXE Prevention
         // https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet
-        SHARED_FACTORY.setExpandEntityReferences(false);
-        SHARED_FACTORY.setXIncludeAware(false);
+
+        SHARED_DOCUMENT_BUILDER_FACTORY.setExpandEntityReferences(false);
+        SHARED_DOCUMENT_BUILDER_FACTORY.setXIncludeAware(false);
 
         trySetFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         trySetFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         trySetFeature("http://xml.org/sax/features/external-general-entities", false);
         trySetFeature("http://xml.org/sax/features/external-parameter-entities", false);
         trySetFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+
+        SHARED_TRANSFORMER_FACTORY.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        SHARED_TRANSFORMER_FACTORY.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
     }
 
     private static void trySetFeature(String feature, boolean value) {
         try {
-            SHARED_FACTORY.setFeature(feature, value);
+            SHARED_DOCUMENT_BUILDER_FACTORY.setFeature(feature, value);
         } catch (Exception e) {
-            LoggerFactory.getLogger(DocumentBuilderUtil.class)
+            LoggerFactory.getLogger(SecureXmlUtil.class)
                 .debug("Error configuring feature: " + feature, e);
         }
     }