From 2bb2baf6f70ec945ddfb0ea300701663f1051a01 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 27 Apr 2021 15:02:15 +0200 Subject: [PATCH] Support more methods that evaluate XPath expressions --- .../src/semmle/code/java/security/XPath.qll | 25 +++--- .../query-tests/security/CWE-643/A.java | 53 ++++++++++++- .../stubs/dom4j-2.1.1/org/dom4j/Document.java | 3 - .../org/dom4j/InvalidXPathException.java | 60 +++++++++++++++ .../stubs/dom4j-2.1.1/org/dom4j/Node.java | 76 +++++++++++++++---- 5 files changed, 185 insertions(+), 32 deletions(-) create mode 100644 java/ql/test/stubs/dom4j-2.1.1/org/dom4j/InvalidXPathException.java diff --git a/java/ql/src/semmle/code/java/security/XPath.qll b/java/ql/src/semmle/code/java/security/XPath.qll index 8b983be5093..a1090e70b26 100644 --- a/java/ql/src/semmle/code/java/security/XPath.qll +++ b/java/ql/src/semmle/code/java/security/XPath.qll @@ -9,13 +9,13 @@ private class XPath extends RefType { XPath() { this.hasQualifiedName("javax.xml.xpath", "XPath") } } -/** A call to `XPath.evaluate` or `XPath.compile` */ -private class XPathEvaluateOrCompile extends MethodAccess { - XPathEvaluateOrCompile() { +/** A call to methods of any class implementing the interface `XPath` that evaluate XPath expressions */ +private class XPathEvaluation extends MethodAccess { + XPathEvaluation() { exists(Method m | - this.getMethod() = m and m.getDeclaringType() instanceof XPath + this.getMethod() = m and m.getDeclaringType().getASourceSupertype*() instanceof XPath | - m.hasName(["evaluate", "compile"]) + m.hasName(["evaluate", "evaluateExpression", "compile"]) ) } } @@ -25,13 +25,16 @@ private class Dom4JNode extends Interface { Dom4JNode() { this.hasQualifiedName("org.dom4j", "Node") } } -/** A call to `Node.selectNodes` or `Node.selectSingleNode` */ -private class NodeSelectNodes extends MethodAccess { - NodeSelectNodes() { +/** A call to methods of any class implementing the interface `Node` that evaluate XPath expressions */ +private class NodeXPathEvaluation extends MethodAccess { + NodeXPathEvaluation() { exists(Method m | this.getMethod() = m and m.getDeclaringType().getASourceSupertype*() instanceof Dom4JNode | - m.hasName(["selectNodes", "selectSingleNode"]) + m.hasName([ + "selectObject", "selectNodes", "selectSingleNode", "numberValueOf", "valueOf", "matches", + "createXPath" + ]) ) } } @@ -44,7 +47,7 @@ abstract class XPathInjectionSink extends DataFlow::Node { } private class DefaultXPathInjectionSink extends XPathInjectionSink { DefaultXPathInjectionSink() { - exists(NodeSelectNodes sink | sink.getArgument(0) = this.asExpr()) or - exists(XPathEvaluateOrCompile sink | sink.getArgument(0) = this.asExpr()) + exists(NodeXPathEvaluation sink | sink.getArgument(0) = this.asExpr()) or + exists(XPathEvaluation sink | sink.getArgument(0) = this.asExpr()) } } diff --git a/java/ql/test/experimental/query-tests/security/CWE-643/A.java b/java/ql/test/experimental/query-tests/security/CWE-643/A.java index e1624124b52..b709a9e7786 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-643/A.java +++ b/java/ql/test/experimental/query-tests/security/CWE-643/A.java @@ -2,27 +2,64 @@ import java.io.ByteArrayInputStream; import java.io.StringReader; import javax.servlet.http.HttpServletRequest; +import javax.xml.namespace.QName; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpression; +import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; import org.w3c.dom.Document; import org.xml.sax.InputSource; public class A { + private static abstract class XPathImplStub implements XPath { + public static XPathImplStub getInstance() { + return null; + } + + @Override + public XPathExpression compile(String expression) throws XPathExpressionException { + return null; + } + + @Override + public Object evaluate(String expression, Object item, QName returnType) throws XPathExpressionException { + return null; + } + + @Override + public String evaluate(String expression, Object item) throws XPathExpressionException { + return null; + } + + @Override + public Object evaluate(String expression, InputSource source, QName returnType) + throws XPathExpressionException { + return null; + } + + @Override + public String evaluate(String expression, InputSource source) throws XPathExpressionException { + return null; + } + + } + public void handle(HttpServletRequest request) throws Exception { final String xmlStr = "" + " " + " " + ""; DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance(); domFactory.setNamespaceAware(true); DocumentBuilder builder = domFactory.newDocumentBuilder(); - Document doc = builder.parse(new InputSource(new StringReader(xmlStr))); + InputSource xmlSource = new InputSource(new StringReader(xmlStr)); + Document doc = builder.parse(xmlSource); XPathFactory factory = XPathFactory.newInstance(); XPath xpath = factory.newXPath(); + XPathImplStub xpathStub = XPathImplStub.getInstance(); // Injectable data String user = request.getParameter("user"); @@ -31,9 +68,13 @@ public class A { // Bad expression String expression1 = "/users/user[@name='" + user + "' and @pass='" + pass + "']"; xpath.evaluate(expression1, doc, XPathConstants.BOOLEAN); // $hasXPathInjection + xpathStub.evaluate(expression1, doc, XPathConstants.BOOLEAN); // $hasXPathInjection + xpath.evaluateExpression(expression1, xmlSource); // $hasXPathInjection + xpathStub.evaluateExpression(expression1, xmlSource); // $hasXPathInjection // Bad expression XPathExpression expression2 = xpath.compile("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + xpathStub.compile("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection expression2.evaluate(doc, XPathConstants.BOOLEAN); // Bad expression @@ -44,6 +85,7 @@ public class A { sb.append("']"); String query = sb.toString(); XPathExpression expression3 = xpath.compile(query); // $hasXPathInjection + xpathStub.compile(query); // $hasXPathInjection expression3.evaluate(doc, XPathConstants.BOOLEAN); // Good expression @@ -63,9 +105,14 @@ public class A { // Bad Dom4j org.dom4j.io.SAXReader reader = new org.dom4j.io.SAXReader(); org.dom4j.Document document = reader.read(new ByteArrayInputStream(xmlStr.getBytes())); - document.selectSingleNode("/users/user[@name='" + user + "' and @pass='" + pass + "']") // $hasXPathInjection - .hasContent(); + document.selectObject("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection document.selectNodes("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + document.selectNodes("/users/user[@name='test']", "/users/user[@pass='" + pass + "']"); // Safe-ish + document.selectSingleNode("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + document.valueOf("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + document.numberValueOf("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + document.matches("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection + document.createXPath("/users/user[@name='" + user + "' and @pass='" + pass + "']"); // $hasXPathInjection } } } \ No newline at end of file diff --git a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Document.java b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Document.java index 5231db0f5cf..a3c430dacb1 100644 --- a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Document.java +++ b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Document.java @@ -17,9 +17,6 @@ import java.util.List; public interface Document extends Branch { - public Node selectSingleNode(String xpathExpression); - - public List selectNodes(String xpathExpression); } /* diff --git a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/InvalidXPathException.java b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/InvalidXPathException.java new file mode 100644 index 00000000000..2068ff3895f --- /dev/null +++ b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/InvalidXPathException.java @@ -0,0 +1,60 @@ +/* + * Copyright 2001-2005 (C) MetaStuff, Ltd. All Rights Reserved. + * + * This software is open source. + * See the bottom of this file for the licence. + */ + +/* + * Adapted from DOM4J version 2.1.1 as available at + * https://search.maven.org/remotecontent?filepath=org/dom4j/dom4j/2.1.1/dom4j-2.1.1-sources.jar + * Only relevant stubs of this file have been retained for test purposes. + */ + +package org.dom4j; + +public class InvalidXPathException extends IllegalArgumentException { + public InvalidXPathException(String xpath) { + } + + public InvalidXPathException(String xpath, String reason) { + } + +} + +/* + * Redistribution and use of this software and associated documentation + * ("Software"), with or without modification, are permitted provided that the + * following conditions are met: + * + * 1. Redistributions of source code must retain copyright statements and + * notices. Redistributions must also contain a copy of this document. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * 3. The name "DOM4J" must not be used to endorse or promote products derived + * from this Software without prior written permission of MetaStuff, Ltd. For + * written permission, please contact dom4j-info@metastuff.com. + * + * 4. Products derived from this Software may not be called "DOM4J" nor may + * "DOM4J" appear in their names without prior written permission of MetaStuff, + * Ltd. DOM4J is a registered trademark of MetaStuff, Ltd. + * + * 5. Due credit should be given to the DOM4J Project - http://www.dom4j.org + * + * THIS SOFTWARE IS PROVIDED BY METASTUFF, LTD. AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL METASTUFF, LTD. OR ITS CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * Copyright 2001-2005 (C) MetaStuff, Ltd. All Rights Reserved. + */ \ No newline at end of file diff --git a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Node.java b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Node.java index 641c5c15f86..34cca235a9d 100644 --- a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Node.java +++ b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/Node.java @@ -6,26 +6,72 @@ */ /* - * Adapted from DOM4J version 2.1.1 as available at - * https://search.maven.org/remotecontent?filepath=org/dom4j/dom4j/2.1.1/dom4j-2.1.1-sources.jar - * Only relevant stubs of this file have been retained for test purposes. - */ +* Adapted from DOM4J version 2.1.1 as available at +* https://search.maven.org/remotecontent?filepath=org/dom4j/dom4j/2.1.1/dom4j-2.1.1-sources.jar +* Only relevant stubs of this file have been retained for test purposes. +*/ + package org.dom4j; -public interface Node { +import java.util.List; + +import javax.xml.xpath.XPath; + +public interface Node extends Cloneable { + + List selectNodes(String xpathExpression); + + Object selectObject(String xpathExpression); + + List selectNodes(String xpathExpression, String comparisonXPathExpression); + + List selectNodes(String xpathExpression, String comparisonXPathExpression, boolean removeDuplicates); + + Node selectSingleNode(String xpathExpression); + + String valueOf(String xpathExpression); + + Number numberValueOf(String xpathExpression); + + boolean matches(String xpathExpression); + + XPath createXPath(String xpathExpression) throws InvalidXPathException; - public boolean hasContent(); } /* + * Redistribution and use of this software and associated documentation + * ("Software"), with or without modification, are permitted provided that the + * following conditions are met: + * + * 1. Redistributions of source code must retain copyright statements and + * notices. Redistributions must also contain a copy of this document. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * 3. The name "DOM4J" must not be used to endorse or promote products derived + * from this Software without prior written permission of MetaStuff, Ltd. For + * written permission, please contact dom4j-info@metastuff.com. + * + * 4. Products derived from this Software may not be called "DOM4J" nor may + * "DOM4J" appear in their names without prior written permission of MetaStuff, + * Ltd. DOM4J is a registered trademark of MetaStuff, Ltd. + * + * 5. Due credit should be given to the DOM4J Project - http://www.dom4j.org + * + * THIS SOFTWARE IS PROVIDED BY METASTUFF, LTD. AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL METASTUFF, LTD. OR ITS CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * * Copyright 2001-2005 (C) MetaStuff, Ltd. All Rights Reserved. - * - * This software is open source. See the bottom of this file for the licence. - */ - -/* - * Adapted from DOM4J version 2.1.1 as available at - * https://search.maven.org/remotecontent?filepath=org/dom4j/dom4j/2.1.1/dom4j-2 - * .1.1-sources.jar Only relevant stubs of this file have been retained for test - * purposes. */ \ No newline at end of file