Fix CVE-2023-24441 preventing XXE attacks (#20)

Configure DocumentBuilderFactory and XSLTransformer so that XML parser disables the use of DTDs
jenkinsci · Jun 16, 2023 · f9b9b0c · f9b9b0c
1 parent 3870f53
commit f9b9b0c
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 1 deletion.
diff --git a/src/main/java/hudson/plugins/mstest/MSTestReportConverter.java b/src/main/java/hudson/plugins/mstest/MSTestReportConverter.java
@@ -8,6 +8,7 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -113,6 +114,14 @@ private void convertToEmma(File f, File c)
     private boolean containsData(File c) throws IOException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
         try {
+            factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+
             DocumentBuilder builder = factory.newDocumentBuilder();
             Document doc = builder.parse(c);
             XPathFactory xPathfactory = XPathFactory.newInstance();
@@ -148,6 +157,12 @@ private DocumentBuilder getDocumentBuilder()
         throws TransformerFactoryConfigurationError,
         ParserConfigurationException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
         return factory.newDocumentBuilder();
     }
 

diff --git a/src/main/java/hudson/plugins/mstest/XslTransformer.java b/src/main/java/hudson/plugins/mstest/XslTransformer.java
@@ -4,6 +4,7 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import javax.xml.XMLConstants;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
@@ -22,12 +23,16 @@ class XslTransformer {
     XslTransformer()
         throws TransformerConfigurationException {
         TransformerFactory transformerFactory = TransformerFactory.newInstance();
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         xslTransformer = transformerFactory.newTransformer();
     }
 
     private XslTransformer(String xslTransform)
         throws TransformerConfigurationException {
         TransformerFactory transformerFactory = TransformerFactory.newInstance();
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         xslTransformer = transformerFactory
             .newTransformer(new StreamSource(this.getClass().getResourceAsStream(xslTransform)));
     }

diff --git a/src/test/java/hudson/plugins/mstest/MSTestReportConverterTest.java b/src/test/java/hudson/plugins/mstest/MSTestReportConverterTest.java
@@ -4,6 +4,9 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
@@ -29,10 +32,19 @@
 public class MSTestReportConverterTest {
 
     @Before
-    public void setUp() {
+    public void setUp() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+
         XMLUnit.setIgnoreWhitespace(true);
         XMLUnit.setNormalizeWhitespace(true);
         XMLUnit.setIgnoreComments(true);
+        XMLUnit.setControlDocumentBuilderFactory(factory);
     }
 
     @Test