Merge branch 'main' into promote-jexl-injection

java/ql/src/Security/CWE/CWE-643/XPathInjection.java

@@ -0,0 +1,83 @@
+final String xmlStr = "<users>" + 
+                        "   <user name=\"aaa\" pass=\"pass1\"></user>" + 
+                        "   <user name=\"bbb\" pass=\"pass2\"></user>" + 
+                        "</users>";
+try {
+    DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance();
+    domFactory.setNamespaceAware(true);
+    DocumentBuilder builder = domFactory.newDocumentBuilder();
+    //Document doc = builder.parse("user.xml");
+    Document doc = builder.parse(new InputSource(new StringReader(xmlStr)));
+
+    XPathFactory factory = XPathFactory.newInstance();
+    XPath xpath = factory.newXPath();
+
+    // Injectable data
+    String user = request.getParameter("user");
+    String pass = request.getParameter("pass");
+    if (user != null && pass != null) {
+        boolean isExist = false;
+
+        // Bad expression
+        String expression1 = "/users/user[@name='" + user + "' and @pass='" + pass + "']";
+        isExist = (boolean)xpath.evaluate(expression1, doc, XPathConstants.BOOLEAN);
+        System.out.println(isExist);
+
+        // Bad expression
+        XPathExpression expression2 = xpath.compile("/users/user[@name='" + user + "' and @pass='" + pass + "']");
+        isExist = (boolean)expression2.evaluate(doc, XPathConstants.BOOLEAN);
+        System.out.println(isExist);
+
+        // Bad expression
+        StringBuffer sb = new StringBuffer("/users/user[@name=");
+        sb.append(user);
+        sb.append("' and @pass='");
+        sb.append(pass);
+        sb.append("']");
+        String query = sb.toString();
+        XPathExpression expression3 = xpath.compile(query);
+        isExist = (boolean)expression3.evaluate(doc, XPathConstants.BOOLEAN);
+        System.out.println(isExist);
+
+        // Good expression
+        String expression4 = "/users/user[@name=$user and @pass=$pass]";
+        xpath.setXPathVariableResolver(v -> {
+        switch (v.getLocalPart()) {
+            case "user":
+                return user;
+            case "pass":
+                return pass;
+            default:
+                throw new IllegalArgumentException();
+            }
+        });
+        isExist = (boolean)xpath.evaluate(expression4, doc, XPathConstants.BOOLEAN);
+        System.out.println(isExist);
+
+
+        // Bad Dom4j 
+        org.dom4j.io.SAXReader reader = new org.dom4j.io.SAXReader();
+        org.dom4j.Document document = reader.read(new InputSource(new StringReader(xmlStr)));
+        isExist = document.selectSingleNode("/users/user[@name='" + user + "' and @pass='" + pass + "']") != null;
+        // or document.selectNodes
+        System.out.println(isExist);
+
+        // Good Dom4j
+        org.jaxen.SimpleVariableContext svc = new org.jaxen.SimpleVariableContext();
+        svc.setVariableValue("user", user);
+        svc.setVariableValue("pass", pass);
+        String xpathString = "/users/user[@name=$user and @pass=$pass]";
+        org.dom4j.XPath safeXPath = document.createXPath(xpathString);
+        safeXPath.setVariableContext(svc);
+        isExist = safeXPath.selectSingleNode(document) != null;
+        System.out.println(isExist);
+    }
+} catch (ParserConfigurationException e) {
+
+} catch (SAXException e) {
+
+} catch (XPathExpressionException e) {
+
+} catch (org.dom4j.DocumentException e) {
+
+}

java/ql/src/Security/CWE/CWE-643/XPathInjection.qhelp

@@ -0,0 +1,44 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+If an XPath expression is built using string concatenation, and the components of the concatenation
+include user input, it makes it very easy for a user to create a malicious XPath expression.
+</p>
+</overview>
+
+<recommendation>
+<p>
+If user input must be included in an XPath expression, either sanitize the data or pre-compile the query
+and use variable references to include the user input.
+</p>
+<p>
+XPath injection can also be prevented by using XQuery.
+</p>
+
+</recommendation>
+
+<example>
+<p>
+In the first three examples, the code accepts a name and password specified by the user, and uses this
+unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing
+special characters or string sequences that change the meaning of the XPath expression to search
+for different values.
+</p>
+
+<p>
+In the fourth example, the code uses <code>setXPathVariableResolver</code> which prevents XPath injection.
+</p>
+<p>
+The final two examples are for dom4j. They show an example of XPath injection and one method of preventing it.
+</p>
+<sample src="XPathInjection.java" />
+</example>
+
+<references>
+<li>OWASP: <a href="https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/07-Input_Validation_Testing/09-Testing_for_XPath_Injection">Testing for XPath Injection</a>.</li>
+<li>OWASP: <a href="https://owasp.org/www-community/attacks/XPATH_Injection">XPath Injection</a>.</li>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-643/XPathInjection.ql

@@ -0,0 +1,30 @@
+/**
+ * @name XPath injection
+ * @description Building an XPath expression from user-controlled sources is vulnerable to insertion of
+ *              malicious code by the user.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/xml/xpath-injection
+ * @tags security
+ *       external/cwe/cwe-643
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.XPath
+import DataFlow::PathGraph
+
+class XPathInjectionConfiguration extends TaintTracking::Configuration {
+  XPathInjectionConfiguration() { this = "XPathInjection" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof XPathInjectionSink }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, XPathInjectionConfiguration c
+where c.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "$@ flows to here and is used in an XPath expression.",
+  source.getNode(), "User-provided value"