Skip to content

Commit 1e6b539

Browse files
aschackmullaschackmull
authored
Merge pull request github#4994 from haby0/main
Java: CWE-652: Improper Neutralization of Data within XQuery Expressions ('XQuery Injection')
2 parents b46a361 + fe046ec commit 1e6b539

28 files changed

+700
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-652/XQueryInjection.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import javax.servlet.http.HttpServletRequest;
2+
import javax.xml.namespace.QName;
3+
import javax.xml.xquery.XQConnection;
4+
import javax.xml.xquery.XQDataSource;
5+
import javax.xml.xquery.XQException;
6+
import javax.xml.xquery.XQItemType;
7+
import javax.xml.xquery.XQPreparedExpression;
8+
import javax.xml.xquery.XQResultSequence;
9+
import net.sf.saxon.xqj.SaxonXQDataSource;
10+
11+
public void bad(HttpServletRequest request) throws XQException {
12+
String name = request.getParameter("name");
13+
XQDataSource ds = new SaxonXQDataSource();
14+
XQConnection conn = ds.getConnection();
15+
String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + "'] return $user/password";
16+
XQPreparedExpression xqpe = conn.prepareExpression(query);
17+
XQResultSequence result = xqpe.executeQuery();
18+
while (result.next()){
19+
System.out.println(result.getItemAsString(null));
20+
}
21+
}
22+
23+
public void bad1(HttpServletRequest request) throws XQException {
24+
String name = request.getParameter("name");
25+
XQDataSource xqds = new SaxonXQDataSource();
26+
String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + "'] return $user/password";
27+
XQConnection conn = xqds.getConnection();
28+
XQExpression expr = conn.createExpression();
29+
XQResultSequence result = expr.executeQuery(query);
30+
while (result.next()){
31+
System.out.println(result.getItemAsString(null));
32+
}
33+
}
34+
35+
public void bad2(HttpServletRequest request) throws XQException {
36+
String name = request.getParameter("name");
37+
XQDataSource xqds = new SaxonXQDataSource();
38+
XQConnection conn = xqds.getConnection();
39+
XQExpression expr = conn.createExpression();
40+
//bad code
41+
expr.executeCommand(name);
42+
}
43+
44+
public void good(HttpServletRequest request) throws XQException {
45+
String name = request.getParameter("name");
46+
XQDataSource ds = new SaxonXQDataSource();
47+
XQConnection conn = ds.getConnection();
48+
String query = "declare variable $name as xs:string external;"
49+
+ " for $user in doc(\"users.xml\")/Users/User[name=$name] return $user/password";
50+
XQPreparedExpression xqpe = conn.prepareExpression(query);
51+
xqpe.bindString(new QName("name"), name, conn.createAtomicType(XQItemType.XQBASETYPE_STRING));
52+
XQResultSequence result = xqpe.executeQuery();
53+
while (result.next()){
54+
System.out.println(result.getItemAsString(null));
55+
}
56+
}
57+
58+
public void good1(HttpServletRequest request) throws XQException {
59+
String name = request.getParameter("name");
60+
String query = "declare variable $name as xs:string external;"
61+
+ " for $user in doc(\"users.xml\")/Users/User[name=$name] return $user/password";
62+
XQDataSource xqds = new SaxonXQDataSource();
63+
XQConnection conn = xqds.getConnection();
64+
XQExpression expr = conn.createExpression();
65+
expr.bindString(new QName("name"), name, conn.createAtomicType(XQItemType.XQBASETYPE_STRING));
66+
XQResultSequence result = expr.executeQuery(query);
67+
while (result.next()){
68+
System.out.println(result.getItemAsString(null));
69+
}
70+
}

‎java/ql/src/experimental/Security/CWE/CWE-652/XQueryInjection.qhelp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The software uses external input to dynamically construct an XQuery expression which is then used to retrieve data from an XML database.
7+
However, the input is not neutralized, or is incorrectly neutralized, which allows an attacker to control the structure of the query.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>Use parameterized queries. This will help ensure the program retains control of the query structure.</p>
13+
14+
</recommendation>
15+
<example>
16+
17+
<p>The following example compares building a query by string concatenation (bad) vs. using <code>bindString</code> to parameterize the query (good).</p>
18+
19+
<sample src="XQueryInjection.java" />
20+
21+
</example>
22+
<references>
23+
24+
<li>Balisage:
25+
<a href="https://www.balisage.net/Proceedings/vol7/html/Vlist02/BalisageVol7-Vlist02.html">XQuery Injection</a>.</li>
26+
27+
28+
29+
<!-- LocalWords: CWE
30+
-->
31+
32+
</references>
33+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-652/XQueryInjection.ql

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @name XQuery query built from user-controlled sources
3+
* @description Building an XQuery query from user-controlled sources is vulnerable to insertion of
4+
* malicious XQuery code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/xquery-injection
9+
* @tags security
10+
* external/cwe/cwe-652
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.FlowSources
15+
import XQueryInjectionLib
16+
import DataFlow::PathGraph
17+
18+
/**
19+
* A taint-tracking configuration tracing flow from remote sources, through an XQuery parser, to its eventual execution.
20+
*/
21+
class XQueryInjectionConfig extends TaintTracking::Configuration {
22+
XQueryInjectionConfig() { this = "XQueryInjectionConfig" }
23+
24+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
25+
26+
override predicate isSink(DataFlow::Node sink) {
27+
sink.asExpr() = any(XQueryPreparedExecuteCall xpec).getPreparedExpression() or
28+
sink.asExpr() = any(XQueryExecuteCall xec).getExecuteQueryArgument() or
29+
sink.asExpr() = any(XQueryExecuteCommandCall xecc).getExecuteCommandArgument()
30+
}
31+
32+
/**
33+
* Holds if taint from the input `pred` to a `prepareExpression` call flows to the returned prepared expression `succ`.
34+
*/
35+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
36+
exists(XQueryParserCall parser | pred.asExpr() = parser.getInput() and succ.asExpr() = parser)
37+
}
38+
}
39+
40+
from DataFlow::PathNode source, DataFlow::PathNode sink, XQueryInjectionConfig conf
41+
where conf.hasFlowPath(source, sink)
42+
select sink.getNode(), source, sink, "XQuery query might include code from $@.", source.getNode(),
43+
"this user input"

‎java/ql/src/experimental/Security/CWE/CWE-652/XQueryInjectionLib.qll

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import java
2+
3+
/** A call to `XQConnection.prepareExpression`. */
4+
class XQueryParserCall extends MethodAccess {
5+
XQueryParserCall() {
6+
exists(Method m |
7+
this.getMethod() = m and
8+
m.getDeclaringType()
9+
.getASourceSupertype*()
10+
.hasQualifiedName("javax.xml.xquery", "XQConnection") and
11+
m.hasName("prepareExpression")
12+
)
13+
}
14+
15+
/**
16+
* Returns the first parameter of the `prepareExpression` method, which provides
17+
* the string, stream or reader to be compiled into a prepared expression.
18+
*/
19+
Expr getInput() { result = this.getArgument(0) }
20+
}
21+
22+
/** A call to `XQPreparedExpression.executeQuery`. */
23+
class XQueryPreparedExecuteCall extends MethodAccess {
24+
XQueryPreparedExecuteCall() {
25+
exists(Method m |
26+
this.getMethod() = m and
27+
m.hasName("executeQuery") and
28+
m.getDeclaringType()
29+
.getASourceSupertype*()
30+
.hasQualifiedName("javax.xml.xquery", "XQPreparedExpression")
31+
)
32+
}
33+
34+
/** Return this prepared expression. */
35+
Expr getPreparedExpression() { result = this.getQualifier() }
36+
}
37+
38+
/** A call to `XQExpression.executeQuery`. */
39+
class XQueryExecuteCall extends MethodAccess {
40+
XQueryExecuteCall() {
41+
exists(Method m |
42+
this.getMethod() = m and
43+
m.hasName("executeQuery") and
44+
m.getDeclaringType()
45+
.getASourceSupertype*()
46+
.hasQualifiedName("javax.xml.xquery", "XQExpression")
47+
)
48+
}
49+
50+
/** Return this execute query argument. */
51+
Expr getExecuteQueryArgument() { result = this.getArgument(0) }
52+
}
53+
54+
/** A call to `XQExpression.executeCommand`. */
55+
class XQueryExecuteCommandCall extends MethodAccess {
56+
XQueryExecuteCommandCall() {
57+
exists(Method m |
58+
this.getMethod() = m and
59+
m.hasName("executeCommand") and
60+
m.getDeclaringType()
61+
.getASourceSupertype*()
62+
.hasQualifiedName("javax.xml.xquery", "XQExpression")
63+
)
64+
}
65+
66+
/** Return this execute command argument. */
67+
Expr getExecuteCommandArgument() { result = this.getArgument(0) }
68+
}

‎java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.expected

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
edges
2+
| XQueryInjection.java:45:23:45:50 | getParameter(...) : String | XQueryInjection.java:51:35:51:38 | xqpe |
3+
| XQueryInjection.java:59:23:59:50 | getParameter(...) : String | XQueryInjection.java:65:53:65:57 | query |
4+
| XQueryInjection.java:73:32:73:59 | nameStr : String | XQueryInjection.java:79:35:79:38 | xqpe |
5+
| XQueryInjection.java:86:33:86:60 | nameStr : String | XQueryInjection.java:92:53:92:57 | query |
6+
| XQueryInjection.java:100:28:100:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:104:35:104:38 | xqpe |
7+
| XQueryInjection.java:112:28:112:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:116:53:116:56 | name |
8+
| XQueryInjection.java:124:28:124:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:129:35:129:38 | xqpe |
9+
| XQueryInjection.java:137:28:137:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:142:53:142:54 | br |
10+
| XQueryInjection.java:150:23:150:50 | getParameter(...) : String | XQueryInjection.java:155:29:155:32 | name |
11+
| XQueryInjection.java:157:26:157:49 | getInputStream(...) : ServletInputStream | XQueryInjection.java:159:29:159:30 | br |
12+
nodes
13+
| XQueryInjection.java:45:23:45:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
14+
| XQueryInjection.java:51:35:51:38 | xqpe | semmle.label | xqpe |
15+
| XQueryInjection.java:59:23:59:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
16+
| XQueryInjection.java:65:53:65:57 | query | semmle.label | query |
17+
| XQueryInjection.java:73:32:73:59 | nameStr : String | semmle.label | nameStr : String |
18+
| XQueryInjection.java:79:35:79:38 | xqpe | semmle.label | xqpe |
19+
| XQueryInjection.java:86:33:86:60 | nameStr : String | semmle.label | nameStr : String |
20+
| XQueryInjection.java:92:53:92:57 | query | semmle.label | query |
21+
| XQueryInjection.java:100:28:100:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
22+
| XQueryInjection.java:104:35:104:38 | xqpe | semmle.label | xqpe |
23+
| XQueryInjection.java:112:28:112:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
24+
| XQueryInjection.java:116:53:116:56 | name | semmle.label | name |
25+
| XQueryInjection.java:124:28:124:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
26+
| XQueryInjection.java:129:35:129:38 | xqpe | semmle.label | xqpe |
27+
| XQueryInjection.java:137:28:137:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
28+
| XQueryInjection.java:142:53:142:54 | br | semmle.label | br |
29+
| XQueryInjection.java:150:23:150:50 | getParameter(...) : String | semmle.label | getParameter(...) : String |
30+
| XQueryInjection.java:155:29:155:32 | name | semmle.label | name |
31+
| XQueryInjection.java:157:26:157:49 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
32+
| XQueryInjection.java:159:29:159:30 | br | semmle.label | br |
33+
#select
34+
| XQueryInjection.java:51:35:51:38 | xqpe | XQueryInjection.java:45:23:45:50 | getParameter(...) : String | XQueryInjection.java:51:35:51:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:45:23:45:50 | getParameter(...) | this user input |
35+
| XQueryInjection.java:65:53:65:57 | query | XQueryInjection.java:59:23:59:50 | getParameter(...) : String | XQueryInjection.java:65:53:65:57 | query | XQuery query might include code from $@. | XQueryInjection.java:59:23:59:50 | getParameter(...) | this user input |
36+
| XQueryInjection.java:79:35:79:38 | xqpe | XQueryInjection.java:73:32:73:59 | nameStr : String | XQueryInjection.java:79:35:79:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:73:32:73:59 | nameStr | this user input |
37+
| XQueryInjection.java:92:53:92:57 | query | XQueryInjection.java:86:33:86:60 | nameStr : String | XQueryInjection.java:92:53:92:57 | query | XQuery query might include code from $@. | XQueryInjection.java:86:33:86:60 | nameStr | this user input |
38+
| XQueryInjection.java:104:35:104:38 | xqpe | XQueryInjection.java:100:28:100:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:104:35:104:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:100:28:100:51 | getInputStream(...) | this user input |
39+
| XQueryInjection.java:116:53:116:56 | name | XQueryInjection.java:112:28:112:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:116:53:116:56 | name | XQuery query might include code from $@. | XQueryInjection.java:112:28:112:51 | getInputStream(...) | this user input |
40+
| XQueryInjection.java:129:35:129:38 | xqpe | XQueryInjection.java:124:28:124:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:129:35:129:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:124:28:124:51 | getInputStream(...) | this user input |
41+
| XQueryInjection.java:142:53:142:54 | br | XQueryInjection.java:137:28:137:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:142:53:142:54 | br | XQuery query might include code from $@. | XQueryInjection.java:137:28:137:51 | getInputStream(...) | this user input |
42+
| XQueryInjection.java:155:29:155:32 | name | XQueryInjection.java:150:23:150:50 | getParameter(...) : String | XQueryInjection.java:155:29:155:32 | name | XQuery query might include code from $@. | XQueryInjection.java:150:23:150:50 | getParameter(...) | this user input |
43+
| XQueryInjection.java:159:29:159:30 | br | XQueryInjection.java:157:26:157:49 | getInputStream(...) : ServletInputStream | XQueryInjection.java:159:29:159:30 | br | XQuery query might include code from $@. | XQueryInjection.java:157:26:157:49 | getInputStream(...) | this user input |

0 commit comments

Comments
 (0)