You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2019/10/01 09:23:57 UTC

[jmeter] 02/04: Bug 63793 - Fix unsecure XML Parsing

This is an automated email from the ASF dual-hosted git repository.

pmouawad pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git

commit d467f5368f65aca65d6ebd81f3d9fb7bec4967b4
Author: pmouawad <p....@ubik-ingenierie.com>
AuthorDate: Tue Oct 1 10:45:56 2019 +0200

    Bug 63793 - Fix unsecure XML Parsing
---
 .../org/apache/jmeter/assertions/XMLAssertion.java     |  6 +++++-
 .../apache/jmeter/assertions/XMLSchemaAssertion.java   |  3 ++-
 .../org/apache/jmeter/gui/action/SchematicView.java    |  2 ++
 .../jmeter/gui/action/template/TemplateManager.java    |  2 ++
 .../main/java/org/apache/jmeter/util/XPathUtil.java    | 18 ++++++++++++------
 .../apache/jmeter/functions/XPathFileContainer.java    |  5 ++++-
 .../protocol/http/proxy/DefaultSamplerCreator.java     |  2 ++
 .../jms/sampler/render/ObjectMessageRenderer.java      |  1 +
 xdocs/changes.xml                                      | 11 +++++++++++
 9 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/src/components/src/main/java/org/apache/jmeter/assertions/XMLAssertion.java b/src/components/src/main/java/org/apache/jmeter/assertions/XMLAssertion.java
index c200b92..4eb9554 100644
--- a/src/components/src/main/java/org/apache/jmeter/assertions/XMLAssertion.java
+++ b/src/components/src/main/java/org/apache/jmeter/assertions/XMLAssertion.java
@@ -22,6 +22,8 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.io.StringReader;
 
+import javax.xml.XMLConstants;
+
 import org.apache.jmeter.samplers.SampleResult;
 import org.apache.jmeter.testelement.AbstractTestElement;
 import org.apache.jmeter.testelement.ThreadListener;
@@ -46,7 +48,9 @@ public class XMLAssertion extends AbstractTestElement implements Serializable, A
         @Override
         protected XMLReader initialValue() {
             try {
-                return XMLReaderFactory.createXMLReader();
+                XMLReader reader = XMLReaderFactory.createXMLReader();
+                reader.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+                return reader;
             } catch (SAXException e) {
                 log.error("Error initializing XMLReader in XMLAssertion", e);
                 return null;
diff --git a/src/components/src/main/java/org/apache/jmeter/assertions/XMLSchemaAssertion.java b/src/components/src/main/java/org/apache/jmeter/assertions/XMLSchemaAssertion.java
index 3336366..b0d7781 100644
--- a/src/components/src/main/java/org/apache/jmeter/assertions/XMLSchemaAssertion.java
+++ b/src/components/src/main/java/org/apache/jmeter/assertions/XMLSchemaAssertion.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.io.StringReader;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -102,7 +103,7 @@ public class XMLSchemaAssertion extends AbstractTestElement implements Serializa
             parserFactory.setNamespaceAware(true);
             parserFactory.setAttribute(JAXP_SCHEMA_LANGUAGE, W3C_XML_SCHEMA);
             parserFactory.setAttribute(JAXP_SCHEMA_SOURCE, xsdFileName);
-
+            parserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             // create a parser:
             DocumentBuilder parser = parserFactory.newDocumentBuilder();
             parser.setErrorHandler(new SAXErrorHandler(result));
diff --git a/src/core/src/main/java/org/apache/jmeter/gui/action/SchematicView.java b/src/core/src/main/java/org/apache/jmeter/gui/action/SchematicView.java
index 5d34c87..12726d1 100644
--- a/src/core/src/main/java/org/apache/jmeter/gui/action/SchematicView.java
+++ b/src/core/src/main/java/org/apache/jmeter/gui/action/SchematicView.java
@@ -33,6 +33,7 @@ import javax.swing.JMenu;
 import javax.swing.JMenuItem;
 import javax.swing.JOptionPane;
 import javax.swing.MenuElement;
+import javax.xml.XMLConstants;
 import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerFactory;
@@ -69,6 +70,7 @@ public class SchematicView extends AbstractAction implements MenuCreator {
                 throws Exception {
             TransformerFactory factory = TransformerFactory.newInstance(
                     "net.sf.saxon.BasicTransformerFactory", Thread.currentThread().getContextClassLoader());
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             Source xslt;
             if (!StringUtils.isEmpty(DEFAULT_XSL_FILE)) {
                 log.info("Will use file {} for Schematic View generation", DEFAULT_XSL_FILE);
diff --git a/src/core/src/main/java/org/apache/jmeter/gui/action/template/TemplateManager.java b/src/core/src/main/java/org/apache/jmeter/gui/action/template/TemplateManager.java
index 4c1699d..b1cfb9f 100644
--- a/src/core/src/main/java/org/apache/jmeter/gui/action/template/TemplateManager.java
+++ b/src/core/src/main/java/org/apache/jmeter/gui/action/template/TemplateManager.java
@@ -24,6 +24,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.TreeMap;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -167,6 +168,7 @@ public class TemplateManager {
         dbf.setNamespaceAware(true);
         dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
         dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         DocumentBuilder bd = dbf.newDocumentBuilder();
         bd.setEntityResolver(new DefaultEntityResolver());
         LoggingErrorHandler errorHandler = new LoggingErrorHandler(log, file);
diff --git a/src/core/src/main/java/org/apache/jmeter/util/XPathUtil.java b/src/core/src/main/java/org/apache/jmeter/util/XPathUtil.java
index 6bb8bc1..b82a32e 100644
--- a/src/core/src/main/java/org/apache/jmeter/util/XPathUtil.java
+++ b/src/core/src/main/java/org/apache/jmeter/util/XPathUtil.java
@@ -29,6 +29,7 @@ import java.nio.charset.StandardCharsets;
 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;
@@ -65,6 +66,9 @@ import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 import org.xml.sax.SAXParseException;
 
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.LoadingCache;
+
 import net.sf.saxon.s9api.Processor;
 import net.sf.saxon.s9api.SaxonApiException;
 import net.sf.saxon.s9api.XPathExecutable;
@@ -72,9 +76,6 @@ import net.sf.saxon.s9api.XPathSelector;
 import net.sf.saxon.s9api.XdmItem;
 import net.sf.saxon.s9api.XdmNode;
 import net.sf.saxon.s9api.XdmValue;
-
-import com.github.benmanes.caffeine.cache.Caffeine;
-import com.github.benmanes.caffeine.cache.LoadingCache;
 /**
  * This class provides a few utility methods for dealing with XML/XPath.
  */
@@ -115,12 +116,13 @@ public class XPathUtil {
      * @return javax.xml.parsers.DocumentBuilderFactory
      */
     private static synchronized DocumentBuilderFactory makeDocumentBuilderFactory(boolean validate, boolean whitespace,
-            boolean namespace) {
+            boolean namespace) throws ParserConfigurationException {
         if (XPathUtil.documentBuilderFactory == null || documentBuilderFactory.isValidating() != validate
                 || documentBuilderFactory.isNamespaceAware() != namespace
                 || documentBuilderFactory.isIgnoringElementContentWhitespace() != whitespace) {
             // configure the document builder factory
             documentBuilderFactory = DocumentBuilderFactory.newInstance();
+            documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             documentBuilderFactory.setValidating(validate);
             documentBuilderFactory.setNamespaceAware(namespace);
             documentBuilderFactory.setIgnoringElementContentWhitespace(whitespace);
@@ -309,7 +311,9 @@ public class XPathUtil {
     private static String getNodeContent(Node node) {
         StringWriter sw = new StringWriter();
         try {
-            Transformer t = TransformerFactory.newInstance().newTransformer();
+            TransformerFactory factory = TransformerFactory.newInstance();
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            Transformer t = factory.newTransformer();
             t.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
             t.transform(new DOMSource(node), new StreamResult(sw));
         } catch (TransformerException e) {
@@ -731,7 +735,9 @@ public class XPathUtil {
      */
     public static String formatXml(String xml){
         try {
-            Transformer serializer= TransformerFactory.newInstance().newTransformer();
+            TransformerFactory factory = TransformerFactory.newInstance();
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            Transformer serializer= factory.newTransformer();
             serializer.setOutputProperty(OutputKeys.INDENT, "yes");
             serializer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
             Source xmlSource = new SAXSource(new InputSource(new StringReader(xml)));
diff --git a/src/functions/src/main/java/org/apache/jmeter/functions/XPathFileContainer.java b/src/functions/src/main/java/org/apache/jmeter/functions/XPathFileContainer.java
index 527dc83..eb4c338 100644
--- a/src/functions/src/main/java/org/apache/jmeter/functions/XPathFileContainer.java
+++ b/src/functions/src/main/java/org/apache/jmeter/functions/XPathFileContainer.java
@@ -23,6 +23,7 @@ import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -67,7 +68,9 @@ public class XPathFileContainer {
         NodeList nl = null;
         try ( FileInputStream fis = new FileInputStream(fileName);
                 BufferedInputStream bis = new BufferedInputStream(fis) ){
-            DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+            DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            DocumentBuilder builder = factory.newDocumentBuilder();
             nl = XPathUtil.selectNodeList(builder.parse(bis), xpath);
             if(log.isDebugEnabled()) {
                 log.debug("found {}", nl.getLength());
diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java
index 9e00f91..8aeb905 100644
--- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java
+++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java
@@ -25,6 +25,7 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Map;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
@@ -237,6 +238,7 @@ public class DefaultSamplerCreator extends AbstractSamplerCreator {
     private static boolean isPotentialXml(String postData) {
         try {
             SAXParserFactory spf = SAXParserFactory.newInstance();
+            spf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             SAXParser saxParser = spf.newSAXParser();
             XMLReader xmlReader = saxParser.getXMLReader();
             ErrorDetectionHandler detectionHandler =
diff --git a/src/protocol/jms/src/main/java/org/apache/jmeter/protocol/jms/sampler/render/ObjectMessageRenderer.java b/src/protocol/jms/src/main/java/org/apache/jmeter/protocol/jms/sampler/render/ObjectMessageRenderer.java
index 255eacf..af0473c 100644
--- a/src/protocol/jms/src/main/java/org/apache/jmeter/protocol/jms/sampler/render/ObjectMessageRenderer.java
+++ b/src/protocol/jms/src/main/java/org/apache/jmeter/protocol/jms/sampler/render/ObjectMessageRenderer.java
@@ -92,6 +92,7 @@ class ObjectMessageRenderer implements MessageRenderer<Serializable> {
     /** Try to determine encoding based on XML prolog, if none <code>null</code> is returned. **/
     protected String findEncoding(String filename) {
         XMLInputFactory factory = XMLInputFactory.newInstance();
+        factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
         try (FileInputStream input = new FileInputStream(filename)) {
             XMLStreamReader reader = factory.createXMLStreamReader(input);
             return reader.getEncoding();
diff --git a/xdocs/changes.xml b/xdocs/changes.xml
index 86fda4e..e1b62c3 100644
--- a/xdocs/changes.xml
+++ b/xdocs/changes.xml
@@ -77,6 +77,16 @@ to view the last release notes of version 5.1.1.
 <ul>
     <li>HTTP(S) Test Script Recorder now appends number at end of names, while previously it added it at beginning. See <bugzilla>63450</bugzilla></li>
     <li>When using XPath Assertion with an XPath expression returning a boolean, <code>True if nothing matches</code> had no effect and always returned true, see <bugzilla>63455</bugzilla></li>
+    <li>XML parsing now refuses unsecure XML, this has impacts on the following features:
+        <ul>
+            <li>XMLAssertion</li>
+            <li>XMLSchemAssertion</li>
+            <li>XPath function</li>
+            <li>XPath 1 &amp; 2 Extractors</li>
+            <li>XPath 1 &amp; 2 Assertions</li>
+        </ul>
+    
+     </li>
 </ul>
 <!-- =================== Improvements =================== -->
 
@@ -234,6 +244,7 @@ to view the last release notes of version 5.1.1.
     <li><bug>63751</bug>Correct a typo in Chinese translations. Reported by Jinliang Wang (wjl31802 at 126.com)</li>
     <li><bug>63723</bug>Distributed testing: JMeter master ends distributed test though some threads still are active</li>
     <li><bug>63614</bug>Distributed testing: Unable to generate Dashboard report at end of load test</li>
+    <li><bug>63793</bug>Fix unsecure XML Parsing</li>
 </ul>
 
  <!--  =================== Thanks =================== -->