Blob Blame History Raw
From 039b3453d17bb5666d4b7a4eacc6a014703416c7 Mon Sep 17 00:00:00 2001
From: Chris Kelley <ckelley@redhat.com>
Date: Fri, 10 Jun 2022 17:25:07 +0100
Subject: [PATCH] Disable access to external entities when parsing XML

This reduces the vulnerability of XML parsers to XXE (XML external
entity) injection.

The best way to prevent XXE is to stop using XML altogether, which we do
plan to do. Until that happens I consider it worthwhile to tighten the
security here though.
---
 .../cms/servlet/csadmin/SecurityDomainProcessor.java     | 6 +++++-
 .../main/java/com/netscape/cmscore/apps/ServerXml.java   | 1 +
 .../main/java/com/netscape/cmsutil/xml/XMLObject.java    | 9 +++++++++
 .../src/test/java/com/netscape/test/TestListener.java    | 5 ++++-
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/base/server/src/main/java/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java b/base/server/src/main/java/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
index bdd485e89a..07fae1ad50 100644
--- a/base/server/src/main/java/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
+++ b/base/server/src/main/java/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
@@ -24,6 +24,7 @@ import java.util.Enumeration;
 import java.util.Locale;
 import java.util.Vector;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
@@ -697,7 +698,10 @@ public class SecurityDomainProcessor extends Processor {
         XMLObject xmlObject = convertDomainInfoToXMLObject(before);
         Document document = xmlObject.getDocument();
 
-        Transformer transformer = TransformerFactory.newInstance().newTransformer();
+        TransformerFactory transformerFactory = TransformerFactory.newInstance();
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+        Transformer transformer = transformerFactory.newTransformer();
         transformer.setOutputProperty(OutputKeys.INDENT, "yes");
         transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "4");
 
diff --git a/base/server/src/main/java/com/netscape/cmscore/apps/ServerXml.java b/base/server/src/main/java/com/netscape/cmscore/apps/ServerXml.java
index 2a02d722a1..d9ac572747 100644
--- a/base/server/src/main/java/com/netscape/cmscore/apps/ServerXml.java
+++ b/base/server/src/main/java/com/netscape/cmscore/apps/ServerXml.java
@@ -41,6 +41,7 @@ public class ServerXml {
         ServerXml serverXml = new ServerXml();
 
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         DocumentBuilder builder = factory.newDocumentBuilder();
         Document document = builder.parse(filename);
 
diff --git a/base/util/src/main/java/com/netscape/cmsutil/xml/XMLObject.java b/base/util/src/main/java/com/netscape/cmsutil/xml/XMLObject.java
index 81fdbf4b2e..1043bcb477 100644
--- a/base/util/src/main/java/com/netscape/cmsutil/xml/XMLObject.java
+++ b/base/util/src/main/java/com/netscape/cmsutil/xml/XMLObject.java
@@ -25,6 +25,7 @@ import java.io.OutputStream;
 import java.io.StringWriter;
 import java.util.Vector;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -56,6 +57,7 @@ public class XMLObject {
     public XMLObject(InputStream s)
             throws SAXException, IOException, ParserConfigurationException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         DocumentBuilder docBuilder = factory.newDocumentBuilder();
         mDoc = docBuilder.parse(s);
     }
@@ -63,6 +65,7 @@ public class XMLObject {
     public XMLObject(File f)
             throws SAXException, IOException, ParserConfigurationException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         DocumentBuilder docBuilder = factory.newDocumentBuilder();
         mDoc = docBuilder.parse(f);
     }
@@ -159,6 +162,8 @@ public class XMLObject {
     public byte[] toByteArray() throws TransformerConfigurationException, TransformerException {
         ByteArrayOutputStream bos = new ByteArrayOutputStream();
         TransformerFactory tranFactory = TransformerFactory.newInstance();
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         Transformer aTransformer = tranFactory.newTransformer();
         Source src = new DOMSource(mDoc);
         Result dest = new StreamResult(bos);
@@ -169,6 +174,8 @@ public class XMLObject {
     public void output(OutputStream os)
             throws TransformerConfigurationException, TransformerException {
         TransformerFactory tranFactory = TransformerFactory.newInstance();
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         Transformer aTransformer = tranFactory.newTransformer();
         Source src = new DOMSource(mDoc);
         Result dest = new StreamResult(os);
@@ -177,6 +184,8 @@ public class XMLObject {
 
     public String toXMLString() throws TransformerConfigurationException, TransformerException {
         TransformerFactory tranFactory = TransformerFactory.newInstance();
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         Transformer transformer = tranFactory.newTransformer();
         Source src = new DOMSource(mDoc);
         StreamResult dest = new StreamResult(new StringWriter());
diff --git a/base/util/src/test/java/com/netscape/test/TestListener.java b/base/util/src/test/java/com/netscape/test/TestListener.java
index 3181d53dc8..ac5d6e0f42 100644
--- a/base/util/src/test/java/com/netscape/test/TestListener.java
+++ b/base/util/src/test/java/com/netscape/test/TestListener.java
@@ -10,6 +10,7 @@ import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.TimeZone;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.transform.OutputKeys;
@@ -22,7 +23,6 @@ import org.junit.runner.Description;
 import org.junit.runner.Result;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunListener;
-
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Text;
@@ -64,9 +64,12 @@ public class TestListener extends RunListener {
         dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
 
         docBuilderFactory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         docBuilder = docBuilderFactory.newDocumentBuilder();
 
         transFactory = TransformerFactory.newInstance();
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        tranFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         trans = transFactory.newTransformer();
         trans.setOutputProperty(OutputKeys.INDENT, "yes");
 
-- 
2.35.1