[SECURITY-1699]

jenkinsci · Apr 3, 2020 · f0ef84c · f0ef84c
1 parent f4e7853
commit f0ef84c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 2 deletions.
diff --git a/src/main/java/io/jenkins/plugins/coverage/adapter/converter/DocumentConverter.java b/src/main/java/io/jenkins/plugins/coverage/adapter/converter/DocumentConverter.java
@@ -20,6 +20,10 @@ public Document convert(T report) throws CoverageException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
         try {
             factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         } catch (ParserConfigurationException e) {
             e.printStackTrace();
         }

diff --git a/src/main/java/io/jenkins/plugins/coverage/adapter/util/XMLUtils.java b/src/main/java/io/jenkins/plugins/coverage/adapter/util/XMLUtils.java
@@ -179,6 +179,10 @@ private Document readXMLtoDocumentWithoutXSD(File file) throws ParserConfigurati
         DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
         documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", false);
+
         DocumentBuilder builder = documentBuilderFactory.newDocumentBuilder();
 
         return builder.parse(file);
@@ -203,10 +207,12 @@ private TransformerFactory newSecureTransformerFactory() {
         TransformerFactory transformerFactory = new TransformerFactoryImpl();
         try {
             transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            transformerFactory.setAttribute("http://saxon.sf.net/feature/parserFeature?uri=http://apache.org/xml/features/disallow-doctype-decl", true);
+            transformerFactory.setAttribute("http://saxon.sf.net/feature/parserFeature?uri=http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            transformerFactory.setAttribute("http://saxon.sf.net/feature/parserFeature?uri=http://xml.org/sax/features/external-general-entities", false);
+            transformerFactory.setAttribute("http://saxon.sf.net/feature/parserFeature?uri=http://xml.org/sax/features/external-parameter-entities", false);
         } catch (TransformerConfigurationException e) {
             e.printStackTrace();
-            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         }
         return transformerFactory;
     }

diff --git a/src/test/java/io/jenkins/plugins/coverage/CoverageCornerCaseTest.java b/src/test/java/io/jenkins/plugins/coverage/CoverageCornerCaseTest.java
@@ -15,6 +15,10 @@
 import java.nio.file.Path;
 import java.util.Objects;
 
+import io.jenkins.plugins.coverage.adapter.util.XMLUtils;
+import java.io.File;
+import org.w3c.dom.Document;
+
 public class CoverageCornerCaseTest {
 
     @Rule
@@ -64,4 +68,16 @@ public void testIfFoundEmptyReport() throws Exception {
     }
 
 
+    @Test
+    public void testPreventXXE() throws Exception {
+        /*
+          Test for SECURITY-1699: if external entities are executed an exception will be thrown
+          as an invalid external entity (unknown protocol foobar) is defined in the supplied XML
+          test file
+       */
+        Document d;
+        File file = new File(getClass().getResource("sec1699.xml").toURI());
+        d = XMLUtils.getInstance().readXMLtoDocument(file);
+    }
+
 }
diff --git a/src/test/resources/io/jenkins/plugins/coverage/sec1699.xml b/src/test/resources/io/jenkins/plugins/coverage/sec1699.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" ?>
+<!DOCTYPE test [ 
+  <!ENTITY xxe SYSTEM "foobar://this.will.except">
+]>
+<coverage branch-rate="0.363636877551020408" line-rate="0.7036379769299024" timestamp="1571654451" version="gcovr 3.3">
+<sources>
+<source>fedepell</source>
+</sources>
+<packages>
+<package branch-rate="0.7142857142857143" complexity="0.0" line-rate="0.9207547169811321" name="...fedepell">
+<classes>
+<class branch-rate="0.36" complexity="0.0" filename="ooops.c" line-rate="0.48484848484848486" name="ooops.c">
+<methods/>
+<lines>
+<line branch="false" hits="5" number="1"/>
+&xxe;
+</lines>
+</class>
+</classes>
+</package>
+</packages>
+</coverage>