From dbb3d458f5488e3d5e71098325f0bfa933bd3d03 Mon Sep 17 00:00:00 2001 From: haby0 Date: Fri, 12 Feb 2021 10:47:41 +0800 Subject: [PATCH] *)add XQExpression.executeCommand(0) sink --- .../Security/CWE/CWE-652/XQueryInjection.java | 9 +++ .../Security/CWE/CWE-652/XQueryInjection.ql | 3 +- .../CWE/CWE-652/XQueryInjectionLib.qll | 16 +++++ .../security/CWE-652/XQueryInjection.expected | 72 ++++++++++--------- .../security/CWE-652/XQueryInjection.java | 63 +++++++++++----- .../net/sf/saxon/xqj/SaxonXQConnection.java | 1 + 6 files changed, 112 insertions(+), 52 deletions(-) diff --git a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.java b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.java index f00df84968c..1b6be030249 100644 --- a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.java +++ b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.java @@ -32,6 +32,15 @@ public void bad1(HttpServletRequest request) throws XQException { } } +public void bad2(HttpServletRequest request) throws XQException { + String name = request.getParameter("name"); + XQDataSource xqds = new SaxonXQDataSource(); + XQConnection conn = xqds.getConnection(); + XQExpression expr = conn.createExpression(); + //bad code + expr.executeCommand(name); +} + public void good(HttpServletRequest request) throws XQException { String name = request.getParameter("name"); XQDataSource ds = new SaxonXQDataSource(); diff --git a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql index 796df6b68da..69617cad629 100644 --- a/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql +++ b/java/ql/src/Security/CWE/CWE-652/XQueryInjection.ql @@ -25,7 +25,8 @@ class XQueryInjectionConfig extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(XQueryPreparedExecuteCall xpec).getPreparedExpression() or - sink.asExpr() = any(XQueryExecuteCall xec).getExecuteQueryArgument() + sink.asExpr() = any(XQueryExecuteCall xec).getExecuteQueryArgument() or + sink.asExpr() = any(XQueryExecuteCommandCall xecc).getExecuteCommandArgument() } /** diff --git a/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll b/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll index 756caf2cd75..2a4019f2c9a 100644 --- a/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll +++ b/java/ql/src/Security/CWE/CWE-652/XQueryInjectionLib.qll @@ -50,3 +50,19 @@ class XQueryExecuteCall extends MethodAccess { /** Return this execute query argument. */ Expr getExecuteQueryArgument() { result = this.getArgument(0) } } + +/** A call to `XQExpression.executeCommand`. */ +class XQueryExecuteCommandCall extends MethodAccess { + XQueryExecuteCommandCall() { + exists(Method m | + this.getMethod() = m and + m.hasName("executeCommand") and + m.getDeclaringType() + .getASourceSupertype*() + .hasQualifiedName("javax.xml.xquery", "XQExpression") + ) + } + + /** Return this execute command argument. */ + Expr getExecuteCommandArgument() { result = this.getArgument(0) } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.expected index e2b04e1e020..b3ae0ff90fb 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.expected @@ -1,35 +1,43 @@ edges -| XQueryInjection.java:42:23:42:50 | getParameter(...) : String | XQueryInjection.java:47:35:47:38 | xqpe | -| XQueryInjection.java:55:23:55:50 | getParameter(...) : String | XQueryInjection.java:60:53:60:57 | query | -| XQueryInjection.java:68:32:68:59 | nameStr : String | XQueryInjection.java:73:35:73:38 | xqpe | -| XQueryInjection.java:80:33:80:60 | nameStr : String | XQueryInjection.java:85:53:85:57 | query | -| XQueryInjection.java:93:28:93:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:97:35:97:38 | xqpe | -| XQueryInjection.java:105:28:105:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:109:53:109:56 | name | -| XQueryInjection.java:117:28:117:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:122:35:122:38 | xqpe | -| XQueryInjection.java:130:28:130:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:135:53:135:54 | br | +| XQueryInjection.java:45:23:45:50 | getParameter(...) : String | XQueryInjection.java:51:35:51:38 | xqpe | +| XQueryInjection.java:59:23:59:50 | getParameter(...) : String | XQueryInjection.java:65:53:65:57 | query | +| XQueryInjection.java:73:32:73:59 | nameStr : String | XQueryInjection.java:79:35:79:38 | xqpe | +| XQueryInjection.java:86:33:86:60 | nameStr : String | XQueryInjection.java:92:53:92:57 | query | +| XQueryInjection.java:100:28:100:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:104:35:104:38 | xqpe | +| XQueryInjection.java:112:28:112:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:116:53:116:56 | name | +| XQueryInjection.java:124:28:124:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:129:35:129:38 | xqpe | +| XQueryInjection.java:137:28:137:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:142:53:142:54 | br | +| XQueryInjection.java:150:23:150:50 | getParameter(...) : String | XQueryInjection.java:155:29:155:32 | name | +| XQueryInjection.java:157:26:157:49 | getInputStream(...) : ServletInputStream | XQueryInjection.java:159:29:159:30 | br | nodes -| XQueryInjection.java:42:23:42:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| XQueryInjection.java:47:35:47:38 | xqpe | semmle.label | xqpe | -| XQueryInjection.java:55:23:55:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| XQueryInjection.java:60:53:60:57 | query | semmle.label | query | -| XQueryInjection.java:68:32:68:59 | nameStr : String | semmle.label | nameStr : String | -| XQueryInjection.java:73:35:73:38 | xqpe | semmle.label | xqpe | -| XQueryInjection.java:80:33:80:60 | nameStr : String | semmle.label | nameStr : String | -| XQueryInjection.java:85:53:85:57 | query | semmle.label | query | -| XQueryInjection.java:93:28:93:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | -| XQueryInjection.java:97:35:97:38 | xqpe | semmle.label | xqpe | -| XQueryInjection.java:105:28:105:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | -| XQueryInjection.java:109:53:109:56 | name | semmle.label | name | -| XQueryInjection.java:117:28:117:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | -| XQueryInjection.java:122:35:122:38 | xqpe | semmle.label | xqpe | -| XQueryInjection.java:130:28:130:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | -| XQueryInjection.java:135:53:135:54 | br | semmle.label | br | +| XQueryInjection.java:45:23:45:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| XQueryInjection.java:51:35:51:38 | xqpe | semmle.label | xqpe | +| XQueryInjection.java:59:23:59:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| XQueryInjection.java:65:53:65:57 | query | semmle.label | query | +| XQueryInjection.java:73:32:73:59 | nameStr : String | semmle.label | nameStr : String | +| XQueryInjection.java:79:35:79:38 | xqpe | semmle.label | xqpe | +| XQueryInjection.java:86:33:86:60 | nameStr : String | semmle.label | nameStr : String | +| XQueryInjection.java:92:53:92:57 | query | semmle.label | query | +| XQueryInjection.java:100:28:100:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XQueryInjection.java:104:35:104:38 | xqpe | semmle.label | xqpe | +| XQueryInjection.java:112:28:112:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XQueryInjection.java:116:53:116:56 | name | semmle.label | name | +| XQueryInjection.java:124:28:124:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XQueryInjection.java:129:35:129:38 | xqpe | semmle.label | xqpe | +| XQueryInjection.java:137:28:137:51 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XQueryInjection.java:142:53:142:54 | br | semmle.label | br | +| XQueryInjection.java:150:23:150:50 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| XQueryInjection.java:155:29:155:32 | name | semmle.label | name | +| XQueryInjection.java:157:26:157:49 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XQueryInjection.java:159:29:159:30 | br | semmle.label | br | #select -| XQueryInjection.java:47:35:47:38 | xqpe | XQueryInjection.java:42:23:42:50 | getParameter(...) : String | XQueryInjection.java:47:35:47:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:42:23:42:50 | getParameter(...) | this user input | -| XQueryInjection.java:60:53:60:57 | query | XQueryInjection.java:55:23:55:50 | getParameter(...) : String | XQueryInjection.java:60:53:60:57 | query | XQuery query might include code from $@. | XQueryInjection.java:55:23:55:50 | getParameter(...) | this user input | -| XQueryInjection.java:73:35:73:38 | xqpe | XQueryInjection.java:68:32:68:59 | nameStr : String | XQueryInjection.java:73:35:73:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:68:32:68:59 | nameStr | this user input | -| XQueryInjection.java:85:53:85:57 | query | XQueryInjection.java:80:33:80:60 | nameStr : String | XQueryInjection.java:85:53:85:57 | query | XQuery query might include code from $@. | XQueryInjection.java:80:33:80:60 | nameStr | this user input | -| XQueryInjection.java:97:35:97:38 | xqpe | XQueryInjection.java:93:28:93:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:97:35:97:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:93:28:93:51 | getInputStream(...) | this user input | -| XQueryInjection.java:109:53:109:56 | name | XQueryInjection.java:105:28:105:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:109:53:109:56 | name | XQuery query might include code from $@. | XQueryInjection.java:105:28:105:51 | getInputStream(...) | this user input | -| XQueryInjection.java:122:35:122:38 | xqpe | XQueryInjection.java:117:28:117:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:122:35:122:38 | xqpe | XQuery query might include code from $@. | XQueryInjection.java:117:28:117:51 | getInputStream(...) | this user input | -| XQueryInjection.java:135:53:135:54 | br | XQueryInjection.java:130:28:130:51 | getInputStream(...) : ServletInputStream | XQueryInjection.java:135:53:135:54 | br | XQuery query might include code from $@. | XQueryInjection.java:130:28:130:51 | getInputStream(...) | this user input | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | diff --git a/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.java b/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.java index 36f829d81bd..d8df8057cc6 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.java +++ b/java/ql/test/experimental/query-tests/security/CWE-652/XQueryInjection.java @@ -1,3 +1,5 @@ +package com.vuln.v2.controller; + import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; @@ -27,9 +29,10 @@ public class XQueryInjection { + " for $user in doc(\"users.xml\")/Users/User[name=$name] return $user/password"; conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); - expr.bindString(new QName("name"), name, conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); + expr.bindString(new QName("name"), name, + conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); XQResultSequence result = expr.executeQuery(query); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } catch (XQException e) { @@ -42,10 +45,11 @@ public class XQueryInjection { String name = request.getParameter("name"); XQDataSource ds = new SaxonXQDataSource(); XQConnection conn = ds.getConnection(); - String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + "'] return $user/password"; + String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + + "'] return $user/password"; XQPreparedExpression xqpe = conn.prepareExpression(query); XQResultSequence result = xqpe.executeQuery(); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -54,11 +58,12 @@ public class XQueryInjection { public void testRequestbad1(HttpServletRequest request) throws Exception { String name = request.getParameter("name"); XQDataSource xqds = new SaxonXQDataSource(); - String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + "'] return $user/password"; + String query = "for $user in doc(\"users.xml\")/Users/User[name='" + name + + "'] return $user/password"; XQConnection conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); XQResultSequence result = expr.executeQuery(query); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -68,10 +73,11 @@ public class XQueryInjection { public void testStringtbad(@RequestParam String nameStr) throws XQException { XQDataSource ds = new SaxonXQDataSource(); XQConnection conn = ds.getConnection(); - String query = "for $user in doc(\"users.xml\")/Users/User[name='" + nameStr + "'] return $user/password"; + String query = "for $user in doc(\"users.xml\")/Users/User[name='" + nameStr + + "'] return $user/password"; XQPreparedExpression xqpe = conn.prepareExpression(query); XQResultSequence result = xqpe.executeQuery(); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -79,11 +85,12 @@ public class XQueryInjection { @RequestMapping public void testStringtbad1(@RequestParam String nameStr) throws XQException { XQDataSource xqds = new SaxonXQDataSource(); - String query = "for $user in doc(\"users.xml\")/Users/User[name='" + nameStr + "'] return $user/password"; + String query = "for $user in doc(\"users.xml\")/Users/User[name='" + nameStr + + "'] return $user/password"; XQConnection conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); XQResultSequence result = expr.executeQuery(query); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -95,7 +102,7 @@ public class XQueryInjection { XQConnection conn = ds.getConnection(); XQPreparedExpression xqpe = conn.prepareExpression(name); XQResultSequence result = xqpe.executeQuery(); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -107,7 +114,7 @@ public class XQueryInjection { XQConnection conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); XQResultSequence result = expr.executeQuery(name); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -120,7 +127,7 @@ public class XQueryInjection { XQConnection conn = ds.getConnection(); XQPreparedExpression xqpe = conn.prepareExpression(br); XQResultSequence result = xqpe.executeQuery(); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -133,11 +140,26 @@ public class XQueryInjection { XQConnection conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); XQResultSequence result = expr.executeQuery(br); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } + @RequestMapping + public void testExecuteCommandbad(HttpServletRequest request) throws Exception { + String name = request.getParameter("name"); + XQDataSource xqds = new SaxonXQDataSource(); + XQConnection conn = xqds.getConnection(); + XQExpression expr = conn.createExpression(); + //bad code + expr.executeCommand(name); + //bad code + InputStream is = request.getInputStream(); + BufferedReader br = new BufferedReader(new InputStreamReader(is)); + expr.executeCommand(br); + expr.close(); + } + @RequestMapping public void good(HttpServletRequest request) throws XQException { String name = request.getParameter("name"); @@ -146,9 +168,10 @@ public class XQueryInjection { String query = "declare variable $name as xs:string external;" + " for $user in doc(\"users.xml\")/Users/User[name=$name] return $user/password"; XQPreparedExpression xqpe = conn.prepareExpression(query); - xqpe.bindString(new QName("name"), name, conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); + xqpe.bindString(new QName("name"), name, + conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); XQResultSequence result = xqpe.executeQuery(); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } @@ -161,10 +184,12 @@ public class XQueryInjection { XQDataSource xqds = new SaxonXQDataSource(); XQConnection conn = xqds.getConnection(); XQExpression expr = conn.createExpression(); - expr.bindString(new QName("name"), name, conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); + expr.bindString(new QName("name"), name, + conn.createAtomicType(XQItemType.XQBASETYPE_STRING)); XQResultSequence result = expr.executeQuery(query); - while (result.next()){ + while (result.next()) { System.out.println(result.getItemAsString(null)); } } -} \ No newline at end of file +} + diff --git a/java/ql/test/stubs/saxon-xqj-9.x/net/sf/saxon/xqj/SaxonXQConnection.java b/java/ql/test/stubs/saxon-xqj-9.x/net/sf/saxon/xqj/SaxonXQConnection.java index 94bd38fa175..0263588e8b4 100644 --- a/java/ql/test/stubs/saxon-xqj-9.x/net/sf/saxon/xqj/SaxonXQConnection.java +++ b/java/ql/test/stubs/saxon-xqj-9.x/net/sf/saxon/xqj/SaxonXQConnection.java @@ -5,6 +5,7 @@ import net.sf.saxon.Configuration; import javax.xml.xquery.XQConnection; import javax.xml.xquery.XQPreparedExpression; import javax.xml.xquery.XQException; +import javax.xml.xquery.XQExpression; import javax.xml.xquery.XQStaticContext; import java.io.InputStream;