Description: Fix CVE-2014-3600: XML External Entity expansion when evaluating XPath expressions. This patch can be removed after upgrading to ActiveMQ 5.10.1 or later. Origin: backport, https://github.com/apache/activemq/commit/b9696ac Bug: https://issues.apache.org/jira/browse/AMQ-5333 --- a/activemq-optional/src/main/java/org/apache/activemq/filter/JAXPXPathEvaluator.java +++ b/activemq-optional/src/main/java/org/apache/activemq/filter/JAXPXPathEvaluator.java @@ -21,11 +21,13 @@ import javax.jms.BytesMessage; import javax.jms.JMSException; import javax.jms.TextMessage; +import javax.xml.parsers.DocumentBuilder; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; +import org.w3c.dom.Document; import org.xml.sax.InputSource; import org.apache.activemq.command.Message; @@ -61,8 +63,9 @@ private boolean evaluate(byte[] data) { try { InputSource inputSource = new InputSource(new ByteArrayInputStream(data)); - return ((Boolean)expression.evaluate(inputSource, XPathConstants.BOOLEAN)).booleanValue(); - } catch (XPathExpressionException e) { + Document inputDocument = builder.parse(inputSource); + return ((Boolean)xpath.evaluate(xpathExpression, inputDocument, XPathConstants.BOOLEAN)).booleanValue(); + } catch (Exception e) { return false; } } @@ -70,8 +73,9 @@ private boolean evaluate(String text) { try { InputSource inputSource = new InputSource(new StringReader(text)); - return ((Boolean)expression.evaluate(inputSource, XPathConstants.BOOLEAN)).booleanValue(); - } catch (XPathExpressionException e) { + Document inputDocument = builder.parse(inputSource); + return ((Boolean)xpath.evaluate(xpathExpression, inputDocument, XPathConstants.BOOLEAN)).booleanValue(); + } catch (Exception e) { return false; } } --- a/activemq-core/src/main/java/org/apache/activemq/filter/XalanXPathEvaluator.java +++ b/activemq-core/src/main/java/org/apache/activemq/filter/XalanXPathEvaluator.java @@ -25,6 +25,8 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathFactory; import org.w3c.dom.Document; import org.w3c.dom.traversal.NodeIterator; @@ -35,13 +37,20 @@ import org.apache.xpath.CachedXPathAPI; import org.apache.xpath.objects.XObject; - public class XalanXPathEvaluator implements XPathExpression.XPathEvaluator { - private final String xpath; - - public XalanXPathEvaluator(String xpath) { - this.xpath = xpath; + private static final XPathFactory FACTORY = XPathFactory.newInstance(); + private final String xpathExpression; + private final DocumentBuilder builder; + private final XPath xpath = FACTORY.newXPath(); + + public XalanXPathEvaluator(String xpathExpression, DocumentBuilder builder) throws Exception { + this.xpathExpression = xpathExpression; + if (builder != null) { + this.builder = builder; + } else { + throw new RuntimeException("No document builder available"); + } } public boolean evaluate(Message m) throws JMSException { @@ -61,22 +70,9 @@ try { InputSource inputSource = new InputSource(new ByteArrayInputStream(data)); - - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); - DocumentBuilder dbuilder = factory.newDocumentBuilder(); - Document doc = dbuilder.parse(inputSource); - - CachedXPathAPI cachedXPathAPI = new CachedXPathAPI(); - XObject result = cachedXPathAPI.eval(doc, xpath); - if (result.bool()) - return true; - else { - NodeIterator iterator = cachedXPathAPI.selectNodeIterator(doc, xpath); - return (iterator.nextNode() != null); - } - - } catch (Throwable e) { + Document inputDocument = builder.parse(inputSource); + return ((Boolean) xpath.evaluate(xpathExpression, inputDocument, XPathConstants.BOOLEAN)).booleanValue(); + } catch (Exception e) { return false; } } @@ -84,28 +80,15 @@ private boolean evaluate(String text) { try { InputSource inputSource = new InputSource(new StringReader(text)); - - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); - DocumentBuilder dbuilder = factory.newDocumentBuilder(); - Document doc = dbuilder.parse(inputSource); - - //An XPath expression could return a true or false value instead of a node. - //eval() is a better way to determine the boolean value of the exp. - //For compliance with legacy behavior where selecting an empty node returns true, - //selectNodeIterator is attempted in case of a failure. - - CachedXPathAPI cachedXPathAPI = new CachedXPathAPI(); - XObject result = cachedXPathAPI.eval(doc, xpath); - if (result.bool()) - return true; - else { - NodeIterator iterator = cachedXPathAPI.selectNodeIterator(doc, xpath); - return (iterator.nextNode() != null); - } - - } catch (Throwable e) { + Document inputDocument = builder.parse(inputSource); + return ((Boolean) xpath.evaluate(xpathExpression, inputDocument, XPathConstants.BOOLEAN)).booleanValue(); + } catch (Exception e) { return false; } } + + @Override + public String toString() { + return xpathExpression; + } } --- a/activemq-core/src/main/java/org/apache/activemq/filter/XPathExpression.java +++ b/activemq-core/src/main/java/org/apache/activemq/filter/XPathExpression.java @@ -19,8 +19,15 @@ import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Properties; import javax.jms.JMSException; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import org.apache.activemq.command.Message; import org.apache.activemq.util.JMSExceptionSupport; @@ -35,8 +42,10 @@ private static final Logger LOG = LoggerFactory.getLogger(XPathExpression.class); private static final String EVALUATOR_SYSTEM_PROPERTY = "org.apache.activemq.XPathEvaluatorClassName"; private static final String DEFAULT_EVALUATOR_CLASS_NAME = XalanXPathEvaluator.class.getName(); + public static final String DOCUMENT_BUILDER_FACTORY_FEATURE = "org.apache.activemq.documentBuilderFactory.feature"; private static final Constructor EVALUATOR_CONSTRUCTOR; + private static DocumentBuilder builder = null; static { String cn = System.getProperty(EVALUATOR_SYSTEM_PROPERTY, DEFAULT_EVALUATOR_CLASS_NAME); @@ -44,6 +53,21 @@ try { try { m = getXPathEvaluatorConstructor(cn); + DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance(); + builderFactory.setNamespaceAware(true); + builderFactory.setIgnoringElementContentWhitespace(true); + builderFactory.setIgnoringComments(true); + try { + // set some reasonable defaults + builderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + builderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + } catch (ParserConfigurationException e) { + LOG.warn("Error setting document builder factory feature", e); + } + // setup the feature from the system property + setupFeatures(builderFactory); + builder = builderFactory.newDocumentBuilder(); } catch (Throwable e) { LOG.warn("Invalid " + XPathEvaluator.class.getName() + " implementation: " + cn + ", reason: " + e, e); cn = DEFAULT_EVALUATOR_CLASS_NAME; @@ -75,12 +99,41 @@ if (!XPathEvaluator.class.isAssignableFrom(c)) { throw new ClassCastException("" + c + " is not an instance of " + XPathEvaluator.class); } - return c.getConstructor(new Class[] {String.class}); + return c.getConstructor(new Class[] {String.class, DocumentBuilder.class}); + } + + protected static void setupFeatures(DocumentBuilderFactory factory) { + Properties properties = System.getProperties(); + List features = new ArrayList(); + for (Map.Entry prop : properties.entrySet()) { + String key = (String) prop.getKey(); + if (key.startsWith(DOCUMENT_BUILDER_FACTORY_FEATURE)) { + String uri = key.split(DOCUMENT_BUILDER_FACTORY_FEATURE + ":")[1]; + Boolean value = Boolean.valueOf((String)prop.getValue()); + try { + factory.setFeature(uri, value); + features.add("feature " + uri + " value " + value); + } catch (ParserConfigurationException e) { + LOG.warn("DocumentBuilderFactory doesn't support the feature {} with value {}, due to {}.", new Object[]{uri, value, e}); + } + } + } + if (features.size() > 0) { + StringBuffer featureString = new StringBuffer(); + // just log the configured feature + for (String feature : features) { + if (featureString.length() != 0) { + featureString.append(", "); + } + featureString.append(feature); + } + } + } private XPathEvaluator createEvaluator(String xpath2) { try { - return (XPathEvaluator)EVALUATOR_CONSTRUCTOR.newInstance(new Object[] {xpath}); + return (XPathEvaluator)EVALUATOR_CONSTRUCTOR.newInstance(new Object[] {xpath, builder}); } catch (InvocationTargetException e) { Throwable cause = e.getCause(); if (cause instanceof RuntimeException) {