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 <[email protected]> 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 & 2 Extractors</li> + <li>XPath 1 & 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 =================== -->
