Skip to content

Commit 30c37ca

Browse files
yoffyoff
committed
Python: model §accumulator
also slightly rearrange the modelling
1 parent 5611bda commit 30c37ca

File tree

3 files changed

+65
-8
lines changed

3 files changed

+65
-8
lines changed

‎python/ql/lib/semmle/python/frameworks/NoSQL.qll

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,16 @@ private module NoSql {
122122
override predicate vulnerableToStrings() { none() }
123123
}
124124

125+
private class MongoCollectionAggregation extends API::CallNode, NoSqlQuery::Range {
126+
MongoCollectionAggregation() { this = mongoCollection().getMember("aggregate").getACall() }
127+
128+
override DataFlow::Node getQuery() { result = this.getParameter(0).getASubscript().asSink() }
129+
130+
override predicate interpretsDict() { any() }
131+
132+
override predicate vulnerableToStrings() { none() }
133+
}
134+
125135
/** The `$where` query operator executes a string as JavaScript. */
126136
private class WhereQueryOperator extends DataFlow::Node, Decoding::Range {
127137
API::Node dictionary;
@@ -143,7 +153,11 @@ private module NoSql {
143153
override predicate mayExecuteInput() { any() }
144154
}
145155

146-
/** The `$function` query operator executes its `body` string as JavaScript. */
156+
/**
157+
* The `$function` query operator executes its `body` string as JavaScript.
158+
*
159+
* See https://www.mongodb.com/docs/manual/reference/operator/aggregation/function/#mongodb-expression-exp.-function
160+
*/
147161
private class FunctionQueryOperator extends DataFlow::Node, Decoding::Range {
148162
API::Node dictionary;
149163
DataFlow::Node query;
@@ -154,8 +168,39 @@ private module NoSql {
154168
.getMember(mongoCollectionMethodName())
155169
.getACall()
156170
.getParameter(0)
157-
.getASubscript*() and
158-
query = dictionary.getSubscript("$function").getSubscript("body").asSink() and
171+
.getASubscript*()
172+
.getSubscript("$function") and
173+
query = dictionary.getSubscript("body").asSink() and
174+
this = dictionary.asSink()
175+
}
176+
177+
override DataFlow::Node getAnInput() { result = query }
178+
179+
override DataFlow::Node getOutput() { result = this }
180+
181+
override string getFormat() { result = "NoSQL" }
182+
183+
override predicate mayExecuteInput() { any() }
184+
}
185+
186+
/**
187+
* The `$accumulator` query operator executes strings in some of its fields as JavaScript.
188+
*
189+
* See https://www.mongodb.com/docs/manual/reference/operator/aggregation/accumulator/#mongodb-group-grp.-accumulator
190+
*/
191+
private class AccumulatorQueryOperator extends DataFlow::Node, Decoding::Range {
192+
API::Node dictionary;
193+
DataFlow::Node query;
194+
195+
AccumulatorQueryOperator() {
196+
dictionary =
197+
mongoCollection()
198+
.getMember("aggregate")
199+
.getACall()
200+
.getParameter(0)
201+
.getASubscript*()
202+
.getSubscript("$accumulator") and
203+
query = dictionary.getSubscript(["init", "accumulate", "merge", "finalize"]).asSink() and
159204
this = dictionary.asSink()
160205
}
161206

‎python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/NoSqlInjection.expected

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ edges
33
| PoC/server.py:1:26:1:32 | GSSA Variable request | PoC/server.py:26:21:26:27 | ControlFlowNode for request |
44
| PoC/server.py:1:26:1:32 | GSSA Variable request | PoC/server.py:42:14:42:20 | ControlFlowNode for request |
55
| PoC/server.py:1:26:1:32 | GSSA Variable request | PoC/server.py:51:14:51:20 | ControlFlowNode for request |
6+
| PoC/server.py:1:26:1:32 | GSSA Variable request | PoC/server.py:76:14:76:20 | ControlFlowNode for request |
67
| PoC/server.py:26:5:26:17 | SSA variable author_string | PoC/server.py:27:25:27:37 | ControlFlowNode for author_string |
78
| PoC/server.py:26:21:26:27 | ControlFlowNode for request | PoC/server.py:26:5:26:17 | SSA variable author_string |
89
| PoC/server.py:27:5:27:10 | SSA variable author | PoC/server.py:30:27:30:44 | ControlFlowNode for Dict |
@@ -13,8 +14,13 @@ edges
1314
| PoC/server.py:46:38:46:67 | ControlFlowNode for BinaryExpr | PoC/server.py:46:27:46:68 | ControlFlowNode for Dict |
1415
| PoC/server.py:51:5:51:10 | SSA variable author | PoC/server.py:53:17:53:70 | ControlFlowNode for BinaryExpr |
1516
| PoC/server.py:51:14:51:20 | ControlFlowNode for request | PoC/server.py:51:5:51:10 | SSA variable author |
16-
| PoC/server.py:53:17:53:70 | ControlFlowNode for BinaryExpr | PoC/server.py:60:37:60:57 | ControlFlowNode for Dict |
17-
| PoC/server.py:60:37:60:57 | ControlFlowNode for Dict | PoC/server.py:60:27:60:58 | ControlFlowNode for Dict |
17+
| PoC/server.py:53:17:53:70 | ControlFlowNode for BinaryExpr | PoC/server.py:60:51:60:56 | ControlFlowNode for search |
18+
| PoC/server.py:60:51:60:56 | ControlFlowNode for search | PoC/server.py:60:27:60:58 | ControlFlowNode for Dict |
19+
| PoC/server.py:76:5:76:10 | SSA variable author | PoC/server.py:79:23:79:101 | ControlFlowNode for BinaryExpr |
20+
| PoC/server.py:76:14:76:20 | ControlFlowNode for request | PoC/server.py:76:5:76:10 | SSA variable author |
21+
| PoC/server.py:79:23:79:101 | ControlFlowNode for BinaryExpr | PoC/server.py:85:37:85:47 | ControlFlowNode for accumulator |
22+
| PoC/server.py:83:5:83:9 | SSA variable group | PoC/server.py:90:29:90:47 | ControlFlowNode for Dict |
23+
| PoC/server.py:85:37:85:47 | ControlFlowNode for accumulator | PoC/server.py:83:5:83:9 | SSA variable group |
1824
| flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_mongoengine_bad.py:1:26:1:32 | GSSA Variable request |
1925
| flask_mongoengine_bad.py:1:26:1:32 | GSSA Variable request | flask_mongoengine_bad.py:19:21:19:27 | ControlFlowNode for request |
2026
| flask_mongoengine_bad.py:1:26:1:32 | GSSA Variable request | flask_mongoengine_bad.py:26:21:26:27 | ControlFlowNode for request |
@@ -108,7 +114,13 @@ nodes
108114
| PoC/server.py:51:14:51:20 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
109115
| PoC/server.py:53:17:53:70 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
110116
| PoC/server.py:60:27:60:58 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
111-
| PoC/server.py:60:37:60:57 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
117+
| PoC/server.py:60:51:60:56 | ControlFlowNode for search | semmle.label | ControlFlowNode for search |
118+
| PoC/server.py:76:5:76:10 | SSA variable author | semmle.label | SSA variable author |
119+
| PoC/server.py:76:14:76:20 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
120+
| PoC/server.py:79:23:79:101 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
121+
| PoC/server.py:83:5:83:9 | SSA variable group | semmle.label | SSA variable group |
122+
| PoC/server.py:85:37:85:47 | ControlFlowNode for accumulator | semmle.label | ControlFlowNode for accumulator |
123+
| PoC/server.py:90:29:90:47 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
112124
| flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
113125
| flask_mongoengine_bad.py:1:26:1:32 | GSSA Variable request | semmle.label | GSSA Variable request |
114126
| flask_mongoengine_bad.py:19:5:19:17 | SSA variable unsafe_search | semmle.label | SSA variable unsafe_search |
@@ -194,6 +206,7 @@ subpaths
194206
| PoC/server.py:30:27:30:44 | ControlFlowNode for Dict | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | PoC/server.py:30:27:30:44 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
195207
| PoC/server.py:46:27:46:68 | ControlFlowNode for Dict | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | PoC/server.py:46:27:46:68 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
196208
| PoC/server.py:60:27:60:58 | ControlFlowNode for Dict | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | PoC/server.py:60:27:60:58 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
209+
| PoC/server.py:90:29:90:47 | ControlFlowNode for Dict | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | PoC/server.py:90:29:90:47 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | PoC/server.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
197210
| flask_mongoengine_bad.py:22:34:22:44 | ControlFlowNode for json_search | flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_mongoengine_bad.py:22:34:22:44 | ControlFlowNode for json_search | This NoSQL query contains an unsanitized $@. | flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
198211
| flask_mongoengine_bad.py:30:39:30:59 | ControlFlowNode for Dict | flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_mongoengine_bad.py:30:39:30:59 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | flask_mongoengine_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
199212
| flask_pymongo_bad.py:14:31:14:51 | ControlFlowNode for Dict | flask_pymongo_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_pymongo_bad.py:14:31:14:51 | ControlFlowNode for Dict | This NoSQL query contains an unsanitized $@. | flask_pymongo_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |

‎python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/PoC/server.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ def by_group():
8787
# Use `" | "a" === "a` as author
8888
# making the query `this.author === "" | "a" === "a"`
8989
# Found by http://127.0.0.1:5000/byGroup?author=%22%20|%20%22a%22%20===%20%22a
90-
post = posts.aggregate([{ "$group": group }]).next() # $ MISSING: result=BAD
91-
app.logger.error("post", post)
90+
post = posts.aggregate([{ "$group": group }]).next() # $ result=BAD
9291
return show_post(post, author)
9392

9493
@app.route('/', methods=['GET'])

0 commit comments

Comments
 (0)