Skip to content

Commit 2a2484e

Browse files
aschackmullaschackmull
authored
Merge pull request github#2800 from SpaceWhite/CWE-643
CWE-643 XPathInjection on java
2 parents 25b9fcf + 300aee3 commit 2a2484e

File tree

3 files changed

+161
-0
lines changed

3 files changed

+161
-0
lines changed

‎java/ql/src/experimental/CWE-643/XPathInjection.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
final String xmlStr = "<users>" +
2+
" <user name=\"aaa\" pass=\"pass1\"></user>" +
3+
" <user name=\"bbb\" pass=\"pass2\"></user>" +
4+
"</users>";
5+
try {
6+
DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance();
7+
domFactory.setNamespaceAware(true);
8+
DocumentBuilder builder = domFactory.newDocumentBuilder();
9+
//Document doc = builder.parse("user.xml");
10+
Document doc = builder.parse(new InputSource(new StringReader(xmlStr)));
11+
12+
XPathFactory factory = XPathFactory.newInstance();
13+
XPath xpath = factory.newXPath();
14+
15+
// Injectable data
16+
String user = request.getParameter("user");
17+
String pass = request.getParameter("pass");
18+
if (user != null && pass != null) {
19+
boolean isExist = false;
20+
21+
// Bad expression
22+
String expression1 = "/users/user[@name='" + user + "' and @pass='" + pass + "']";
23+
isExist = (boolean)xpath.evaluate(expression1, doc, XPathConstants.BOOLEAN);
24+
System.out.println(isExist);
25+
26+
// Bad expression
27+
XPathExpression expression2 = xpath.compile("/users/user[@name='" + user + "' and @pass='" + pass + "']");
28+
isExist = (boolean)expression2.evaluate(doc, XPathConstants.BOOLEAN);
29+
System.out.println(isExist);
30+
31+
// Bad expression
32+
StringBuffer sb = new StringBuffer("/users/user[@name=");
33+
sb.append(user);
34+
sb.append("' and @pass='");
35+
sb.append(pass);
36+
sb.append("']");
37+
String query = sb.toString();
38+
XPathExpression expression3 = xpath.compile(query);
39+
isExist = (boolean)expression3.evaluate(doc, XPathConstants.BOOLEAN);
40+
System.out.println(isExist);
41+
42+
// Good expression
43+
String expression4 = "/users/user[@name=$user and @pass=$pass]";
44+
xpath.setXPathVariableResolver(v -> {
45+
switch (v.getLocalPart()) {
46+
case "user":
47+
return user;
48+
case "pass":
49+
return pass;
50+
default:
51+
throw new IllegalArgumentException();
52+
}
53+
});
54+
isExist = (boolean)xpath.evaluate(expression4, doc, XPathConstants.BOOLEAN);
55+
System.out.println(isExist);
56+
57+
58+
// Bad Dom4j
59+
org.dom4j.io.SAXReader reader = new org.dom4j.io.SAXReader();
60+
org.dom4j.Document document = reader.read(new InputSource(new StringReader(xmlStr)));
61+
isExist = document.selectSingleNode("/users/user[@name='" + user + "' and @pass='" + pass + "']").hasContent();
62+
// or document.selectNodes
63+
System.out.println(isExist);
64+
}
65+
} catch (ParserConfigurationException e) {
66+
67+
} catch (SAXException e) {
68+
69+
} catch (XPathExpressionException e) {
70+
71+
} catch (org.dom4j.DocumentException e) {
72+
73+
}

‎java/ql/src/experimental/CWE-643/XPathInjection.qhelp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
If an XPath expression is built using string concatenation, and the components of the concatenation
8+
include user input, a user is likely to be able to create a malicious XPath expression.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
If user input must be included in an XPath expression, pre-compile the query and use variable
15+
references to include the user input.
16+
</p>
17+
<p>
18+
XPath injection can also be prevented by using XQuery.
19+
</p>
20+
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
In the first, second, and third example, the code accepts a name and password specified by the user, and uses this
26+
unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing
27+
special characters or string sequences that change the meaning of the XPath expression to search
28+
for different values.
29+
</p>
30+
31+
<p>
32+
In the fourth example, the code utilizes setXPathVariableResolver which prevents XPath Injection.
33+
</p>
34+
<p>
35+
The fifth example is a dom4j XPath injection example.
36+
</p>
37+
<sample src="XPathInjection.java" />
38+
</example>
39+
40+
<references>
41+
<li>OWASP: <a href="https://www.owasp.org/index.php?title=Testing_for_XPath_Injection_(OTG-INPVAL-010)">Testing for XPath Injection</a>.</li>
42+
<li>OWASP: <a href="https://www.owasp.org/index.php/XPATH_Injection">XPath Injection</a>.</li>
43+
</references>
44+
</qhelp>

‎java/ql/src/experimental/CWE-643/XPathInjection.ql

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* @name XPath injection
3+
* @description Building an XPath expression from user-controlled sources is vulnerable to insertion of
4+
* malicious code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/xml/xpath-injection
9+
* @tags security
10+
* external/cwe/cwe-643
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.FlowSources
15+
import semmle.code.java.dataflow.TaintTracking
16+
import semmle.code.java.security.XmlParsers
17+
import DataFlow::PathGraph
18+
19+
class XPathInjectionConfiguration extends TaintTracking::Configuration {
20+
XPathInjectionConfiguration() { this = "XPathInjection" }
21+
22+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof XPathInjectionSink }
25+
}
26+
27+
class XPathInjectionSink extends DataFlow::ExprNode {
28+
XPathInjectionSink() {
29+
exists(Method m, MethodAccess ma | ma.getMethod() = m |
30+
m.getDeclaringType().hasQualifiedName("javax.xml.xpath", "XPath") and
31+
(m.hasName("evaluate") or m.hasName("compile")) and
32+
ma.getArgument(0) = this.getExpr()
33+
or
34+
m.getDeclaringType().hasQualifiedName("org.dom4j", "Node") and
35+
(m.hasName("selectNodes") or m.hasName("selectSingleNode")) and
36+
ma.getArgument(0) = this.getExpr()
37+
)
38+
}
39+
}
40+
41+
from DataFlow::PathNode source, DataFlow::PathNode sink, XPathInjectionConfiguration c
42+
where c.hasFlowPath(source, sink)
43+
select sink.getNode(), source, sink, "$@ flows to here and is used in an XPath expression.",
44+
source.getNode(), "User-provided value"

0 commit comments

Comments
 (0)