From 039b3453d17bb5666d4b7a4eacc6a014703416c7 Mon Sep 17 00:00:00 2001 From: Chris Kelley 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