145 lines
7.5 KiB
Diff
145 lines
7.5 KiB
Diff
From 1fe34b30ed12710f6ea4c2fae4686f36dd4ef705 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
|
|
|
|
Origin: https://github.com/dogtagpki/pki/commit/1fe34b30ed12710f6ea4c2fae4686f36dd4ef705
|
|
|
|
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 +++++-
|
|
.../cmscore/src/com/netscape/cmscore/apps/ServerXml.java | 1 +
|
|
base/test/src/com/netscape/test/TestListener.java | 5 ++++-
|
|
base/util/src/com/netscape/cmsutil/xml/XMLObject.java | 9 +++++++++
|
|
4 files changed, 19 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java b/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
|
|
index 2090fec357a..6931fa5c5f5 100644
|
|
--- a/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
|
|
+++ b/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
|
|
@@ -24,6 +24,7 @@
|
|
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;
|
|
@@ -640,7 +641,10 @@ public static void main(String args[]) throws Exception {
|
|
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/cmscore/src/com/netscape/cmscore/apps/ServerXml.java b/base/server/cmscore/src/com/netscape/cmscore/apps/ServerXml.java
|
|
index 59a06ba39ba..2886291af2d 100644
|
|
--- a/base/server/cmscore/src/com/netscape/cmscore/apps/ServerXml.java
|
|
+++ b/base/server/cmscore/src/com/netscape/cmscore/apps/ServerXml.java
|
|
@@ -40,6 +40,7 @@ public static ServerXml load(String filename) throws Exception {
|
|
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/test/src/com/netscape/test/TestListener.java b/base/test/src/com/netscape/test/TestListener.java
|
|
index 96c4c906892..d55458716fe 100644
|
|
--- a/base/test/src/com/netscape/test/TestListener.java
|
|
+++ b/base/test/src/com/netscape/test/TestListener.java
|
|
@@ -10,6 +10,7 @@
|
|
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.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 TestListener() throws Exception {
|
|
dateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
|
|
|
|
docBuilderFactory = DocumentBuilderFactory.newInstance();
|
|
+ docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
|
|
docBuilder = docBuilderFactory.newDocumentBuilder();
|
|
|
|
transFactory = TransformerFactory.newInstance();
|
|
+ transFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
|
|
+ transFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
|
|
trans = transFactory.newTransformer();
|
|
trans.setOutputProperty(OutputKeys.INDENT, "yes");
|
|
|
|
diff --git a/base/util/src/com/netscape/cmsutil/xml/XMLObject.java b/base/util/src/com/netscape/cmsutil/xml/XMLObject.java
|
|
index a7715ec9908..d8e0f413325 100644
|
|
--- a/base/util/src/com/netscape/cmsutil/xml/XMLObject.java
|
|
+++ b/base/util/src/com/netscape/cmsutil/xml/XMLObject.java
|
|
@@ -25,6 +25,7 @@
|
|
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 XMLObject() throws ParserConfigurationException {
|
|
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 XMLObject(InputStream s)
|
|
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 Vector<String> getValuesFromContainer(Node container, String tagname) {
|
|
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 byte[] toByteArray() throws TransformerConfigurationException, Transforme
|
|
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 void output(OutputStream os)
|
|
|
|
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());
|