Disable external entity processing in XML parsing to prevent XXE vuln…

…erabilities, protecting against data exposure, resource exhaustion, and SSRF attacks.
descoped · Oct 3, 2024 · d89001a · d89001a
1 parent e2f8472
commit d89001a
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 2 deletions.
diff --git a/src/main/java/io/descoped/dc/core/handler/XPathParser.java b/src/main/java/io/descoped/dc/core/handler/XPathParser.java
@@ -12,6 +12,7 @@
 import org.xml.sax.SAXException;
 import org.xml.sax.XMLReader;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -42,6 +43,8 @@ public XPathParser() {
     static XMLReader createSAXFactory() {
         try {
             SAXParserFactory sax = SAXParserFactory.newInstance();
+            // Disable external entities
+            sax.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
             sax.setNamespaceAware(false);
             return sax.newSAXParser().getXMLReader();
         } catch (SAXException | ParserConfigurationException e) {
@@ -52,6 +55,13 @@ static XMLReader createSAXFactory() {
     static DocumentBuilder createDocumentBuilder() {
         try {
             DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+
+            // Disable external entities
+            documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            documentBuilderFactory.setExpandEntityReferences(false);
+
             documentBuilderFactory.setNamespaceAware(false);
             return documentBuilderFactory.newDocumentBuilder();
         } catch (ParserConfigurationException e) {
@@ -62,7 +72,14 @@ static DocumentBuilder createDocumentBuilder() {
     @Override
     public byte[] serialize(Object document) {
         try (StringWriter writer = new StringWriter()) {
-            Transformer transformer = TransformerFactory.newInstance().newTransformer();
+            TransformerFactory transformerFactory = TransformerFactory.newInstance();
+            transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+
+            // Disable external DTDs and stylesheets
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+
+            Transformer transformer = transformerFactory.newTransformer();
             transformer.transform(new DOMSource((Node) document), new StreamResult(writer));
             return writer.toString().getBytes();
         } catch (RuntimeException | Error e) {

diff --git a/src/main/java/io/descoped/dc/core/handler/XmlTokenParser.java b/src/main/java/io/descoped/dc/core/handler/XmlTokenParser.java
@@ -10,6 +10,7 @@
 import org.xml.sax.SAXException;
 import org.xml.sax.XMLReader;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -38,7 +39,14 @@ public XmlTokenParser() {
     @Override
     public byte[] serialize(Object document) {
         try (StringWriter writer = new StringWriter()) {
-            Transformer transformer = TransformerFactory.newInstance().newTransformer();
+            TransformerFactory transformerFactory = TransformerFactory.newInstance();
+            transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+
+            // Disable external DTDs and stylesheets
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+
+            Transformer transformer = transformerFactory.newTransformer();
             transformer.transform(new DOMSource((Node) document), new StreamResult(writer));
             return writer.toString().getBytes();
         } catch (RuntimeException | Error e) {
@@ -51,6 +59,8 @@ public byte[] serialize(Object document) {
     static XMLReader createSAXFactory() {
         try {
             SAXParserFactory sax = SAXParserFactory.newInstance();
+            // Disable external entities
+            sax.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
             sax.setNamespaceAware(false);
             return sax.newSAXParser().getXMLReader();
         } catch (SAXException | ParserConfigurationException e) {
@@ -61,6 +71,13 @@ static XMLReader createSAXFactory() {
     static DocumentBuilder createDocumentBuilder() {
         try {
             DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+
+            // Disable external entities
+            documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            documentBuilderFactory.setExpandEntityReferences(false);
+
             documentBuilderFactory.setNamespaceAware(false);
             return documentBuilderFactory.newDocumentBuilder();
         } catch (ParserConfigurationException e) {

diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java
@@ -31,7 +31,9 @@
     requires commons.jexl3;
     requires com.fasterxml.jackson.databind;
     requires com.fasterxml.jackson.annotation;
+
     requires java.xml;
+
     requires io.github.classgraph;
     requires org.bouncycastle.pkix;
     requires org.bouncycastle.provider;