Skip to content

Commit 2421076

Browse files
RasmusWLRasmusWL
authored
Merge pull request #8696 from RasmusWL/new-nosql-examples
Python: Improve experimental modeling for `pymongo`
2 parents 7aa3d0f + 3c1a37e commit 2421076

File tree

5 files changed

+109
-72
lines changed

5 files changed

+109
-72
lines changed

‎python/ql/src/experimental/semmle/python/frameworks/NoSQL.qll

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ private module NoSql {
1515
/** Gets a reference to `pymongo.MongoClient` */
1616
private API::Node pyMongo() {
1717
result = API::moduleImport("pymongo").getMember("MongoClient").getReturn()
18+
or
19+
// see https://pymongo.readthedocs.io/en/stable/api/pymongo/mongo_client.html#pymongo.mongo_client.MongoClient
20+
result =
21+
API::moduleImport("pymongo").getMember("mongo_client").getMember("MongoClient").getReturn()
1822
}
1923

2024
/** Gets a reference to `flask_pymongo.PyMongo` */
@@ -34,40 +38,36 @@ private module NoSql {
3438
* Gets a reference to an initialized `Mongo` instance.
3539
* See `pyMongo()`, `flask_PyMongo()`
3640
*/
37-
private API::Node mongoInstance() {
41+
private API::Node mongoClientInstance() {
3842
result = pyMongo() or
3943
result = flask_PyMongo()
4044
}
4145

4246
/**
43-
* Gets a reference to an initialized `Mongo` DB instance.
44-
* See `mongoEngine()`, `flask_MongoEngine()`
45-
*/
46-
private API::Node mongoDBInstance() {
47-
result = mongoEngine().getMember(["get_db", "connect"]).getReturn() or
48-
result = mongoEngine().getMember("connection").getMember(["get_db", "connect"]).getReturn() or
49-
result = flask_MongoEngine().getMember("get_db").getReturn()
50-
}
51-
52-
/**
53-
* Gets a reference to a `Mongo` DB use.
54-
*
55-
* See `mongoInstance()`, `mongoDBInstance()`.
47+
* Gets a reference to a `Mongo` DB instance.
5648
*/
57-
private DataFlow::LocalSourceNode mongoDB(DataFlow::TypeTracker t) {
49+
private DataFlow::LocalSourceNode mongoDBInstance(DataFlow::TypeTracker t) {
5850
t.start() and
5951
(
6052
exists(SubscriptNode subscript |
61-
subscript.getObject() = mongoInstance().getAUse().asCfgNode() and
53+
subscript.getObject() = mongoClientInstance().getAUse().asCfgNode() and
6254
result.asCfgNode() = subscript
6355
)
6456
or
65-
result.(DataFlow::AttrRead).getObject() = mongoInstance().getAUse()
57+
result.(DataFlow::AttrRead).getObject() = mongoClientInstance().getAUse()
6658
or
67-
result = mongoDBInstance().getAnImmediateUse()
59+
result = mongoEngine().getMember(["get_db", "connect"]).getACall()
60+
or
61+
result = mongoEngine().getMember("connection").getMember(["get_db", "connect"]).getACall()
62+
or
63+
result = flask_MongoEngine().getMember("get_db").getACall()
64+
or
65+
// see https://pymongo.readthedocs.io/en/stable/api/pymongo/mongo_client.html#pymongo.mongo_client.MongoClient.get_default_database
66+
// see https://pymongo.readthedocs.io/en/stable/api/pymongo/mongo_client.html#pymongo.mongo_client.MongoClient.get_database
67+
result = mongoClientInstance().getMember(["get_default_database", "get_database"]).getACall()
6868
)
6969
or
70-
exists(DataFlow::TypeTracker t2 | result = mongoDB(t2).track(t2, t))
70+
exists(DataFlow::TypeTracker t2 | result = mongoDBInstance(t2).track(t2, t))
7171
}
7272

7373
/**
@@ -81,21 +81,27 @@ private module NoSql {
8181
*
8282
* `mongo.db` would be a use of a `Mongo` instance, and so the result.
8383
*/
84-
private DataFlow::Node mongoDB() { mongoDB(DataFlow::TypeTracker::end()).flowsTo(result) }
84+
private DataFlow::Node mongoDBInstance() {
85+
mongoDBInstance(DataFlow::TypeTracker::end()).flowsTo(result)
86+
}
8587

8688
/**
8789
* Gets a reference to a `Mongo` collection use.
88-
*
89-
* See `mongoDB()`.
9090
*/
9191
private DataFlow::LocalSourceNode mongoCollection(DataFlow::TypeTracker t) {
9292
t.start() and
9393
(
9494
exists(SubscriptNode subscript | result.asCfgNode() = subscript |
95-
subscript.getObject() = mongoDB().asCfgNode()
95+
subscript.getObject() = mongoDBInstance().asCfgNode()
9696
)
9797
or
98-
result.(DataFlow::AttrRead).getObject() = mongoDB()
98+
result.(DataFlow::AttrRead).getObject() = mongoDBInstance()
99+
or
100+
// see https://pymongo.readthedocs.io/en/stable/api/pymongo/database.html#pymongo.database.Database.get_collection
101+
// see https://pymongo.readthedocs.io/en/stable/api/pymongo/database.html#pymongo.database.Database.create_collection
102+
result
103+
.(DataFlow::MethodCallNode)
104+
.calls(mongoDBInstance(), ["get_collection", "create_collection"])
99105
)
100106
or
101107
exists(DataFlow::TypeTracker t2 | result = mongoCollection(t2).track(t2, t))

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

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,19 @@ edges
4444
| mongoengine_bad.py:57:21:57:42 | ControlFlowNode for Subscript | mongoengine_bad.py:58:30:58:42 | ControlFlowNode for unsafe_search |
4545
| mongoengine_bad.py:58:19:58:43 | ControlFlowNode for Attribute() | mongoengine_bad.py:61:29:61:49 | ControlFlowNode for Dict |
4646
| mongoengine_bad.py:58:30:58:42 | ControlFlowNode for unsafe_search | mongoengine_bad.py:58:19:58:43 | ControlFlowNode for Attribute() |
47-
| pymongo_bad.py:11:21:11:27 | ControlFlowNode for request | pymongo_bad.py:11:21:11:32 | ControlFlowNode for Attribute |
48-
| pymongo_bad.py:11:21:11:32 | ControlFlowNode for Attribute | pymongo_bad.py:11:21:11:42 | ControlFlowNode for Subscript |
49-
| pymongo_bad.py:11:21:11:42 | ControlFlowNode for Subscript | pymongo_bad.py:12:30:12:42 | ControlFlowNode for unsafe_search |
50-
| pymongo_bad.py:12:19:12:43 | ControlFlowNode for Attribute() | pymongo_bad.py:14:42:14:62 | ControlFlowNode for Dict |
51-
| pymongo_bad.py:12:30:12:42 | ControlFlowNode for unsafe_search | pymongo_bad.py:12:19:12:43 | ControlFlowNode for Attribute() |
47+
| pymongo_test.py:12:21:12:27 | ControlFlowNode for request | pymongo_test.py:12:21:12:32 | ControlFlowNode for Attribute |
48+
| pymongo_test.py:12:21:12:32 | ControlFlowNode for Attribute | pymongo_test.py:12:21:12:42 | ControlFlowNode for Subscript |
49+
| pymongo_test.py:12:21:12:42 | ControlFlowNode for Subscript | pymongo_test.py:13:30:13:42 | ControlFlowNode for unsafe_search |
50+
| pymongo_test.py:13:19:13:43 | ControlFlowNode for Attribute() | pymongo_test.py:15:42:15:62 | ControlFlowNode for Dict |
51+
| pymongo_test.py:13:30:13:42 | ControlFlowNode for unsafe_search | pymongo_test.py:13:19:13:43 | ControlFlowNode for Attribute() |
52+
| pymongo_test.py:29:16:29:51 | ControlFlowNode for Attribute() | pymongo_test.py:33:34:33:73 | ControlFlowNode for Dict |
53+
| pymongo_test.py:29:27:29:33 | ControlFlowNode for request | pymongo_test.py:29:27:29:38 | ControlFlowNode for Attribute |
54+
| pymongo_test.py:29:27:29:38 | ControlFlowNode for Attribute | pymongo_test.py:29:27:29:50 | ControlFlowNode for Subscript |
55+
| pymongo_test.py:29:27:29:50 | ControlFlowNode for Subscript | pymongo_test.py:29:16:29:51 | ControlFlowNode for Attribute() |
56+
| pymongo_test.py:39:16:39:51 | ControlFlowNode for Attribute() | pymongo_test.py:43:34:43:73 | ControlFlowNode for Dict |
57+
| pymongo_test.py:39:27:39:33 | ControlFlowNode for request | pymongo_test.py:39:27:39:38 | ControlFlowNode for Attribute |
58+
| pymongo_test.py:39:27:39:38 | ControlFlowNode for Attribute | pymongo_test.py:39:27:39:50 | ControlFlowNode for Subscript |
59+
| pymongo_test.py:39:27:39:50 | ControlFlowNode for Subscript | pymongo_test.py:39:16:39:51 | ControlFlowNode for Attribute() |
5260
nodes
5361
| flask_mongoengine_bad.py:19:21:19:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
5462
| flask_mongoengine_bad.py:19:21:19:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
@@ -104,12 +112,22 @@ nodes
104112
| mongoengine_bad.py:58:19:58:43 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
105113
| mongoengine_bad.py:58:30:58:42 | ControlFlowNode for unsafe_search | semmle.label | ControlFlowNode for unsafe_search |
106114
| mongoengine_bad.py:61:29:61:49 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
107-
| pymongo_bad.py:11:21:11:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
108-
| pymongo_bad.py:11:21:11:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
109-
| pymongo_bad.py:11:21:11:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
110-
| pymongo_bad.py:12:19:12:43 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
111-
| pymongo_bad.py:12:30:12:42 | ControlFlowNode for unsafe_search | semmle.label | ControlFlowNode for unsafe_search |
112-
| pymongo_bad.py:14:42:14:62 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
115+
| pymongo_test.py:12:21:12:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
116+
| pymongo_test.py:12:21:12:32 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
117+
| pymongo_test.py:12:21:12:42 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
118+
| pymongo_test.py:13:19:13:43 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
119+
| pymongo_test.py:13:30:13:42 | ControlFlowNode for unsafe_search | semmle.label | ControlFlowNode for unsafe_search |
120+
| pymongo_test.py:15:42:15:62 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
121+
| pymongo_test.py:29:16:29:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
122+
| pymongo_test.py:29:27:29:33 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
123+
| pymongo_test.py:29:27:29:38 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
124+
| pymongo_test.py:29:27:29:50 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
125+
| pymongo_test.py:33:34:33:73 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
126+
| pymongo_test.py:39:16:39:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
127+
| pymongo_test.py:39:27:39:33 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
128+
| pymongo_test.py:39:27:39:38 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
129+
| pymongo_test.py:39:27:39:50 | ControlFlowNode for Subscript | semmle.label | ControlFlowNode for Subscript |
130+
| pymongo_test.py:43:34:43:73 | ControlFlowNode for Dict | semmle.label | ControlFlowNode for Dict |
113131
subpaths
114132
#select
115133
| flask_mongoengine_bad.py:22:34:22:44 | ControlFlowNode for json_search | flask_mongoengine_bad.py:19:21:19:27 | ControlFlowNode for request | flask_mongoengine_bad.py:22:34:22:44 | ControlFlowNode for json_search | $@ NoSQL query contains an unsanitized $@ | flask_mongoengine_bad.py:22:34:22:44 | ControlFlowNode for json_search | This | flask_mongoengine_bad.py:19:21:19:27 | ControlFlowNode for request | user-provided value |
@@ -121,4 +139,6 @@ subpaths
121139
| mongoengine_bad.py:46:26:46:46 | ControlFlowNode for Dict | mongoengine_bad.py:42:21:42:27 | ControlFlowNode for request | mongoengine_bad.py:46:26:46:46 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | mongoengine_bad.py:46:26:46:46 | ControlFlowNode for Dict | This | mongoengine_bad.py:42:21:42:27 | ControlFlowNode for request | user-provided value |
122140
| mongoengine_bad.py:53:34:53:44 | ControlFlowNode for json_search | mongoengine_bad.py:50:21:50:27 | ControlFlowNode for request | mongoengine_bad.py:53:34:53:44 | ControlFlowNode for json_search | $@ NoSQL query contains an unsanitized $@ | mongoengine_bad.py:53:34:53:44 | ControlFlowNode for json_search | This | mongoengine_bad.py:50:21:50:27 | ControlFlowNode for request | user-provided value |
123141
| mongoengine_bad.py:61:29:61:49 | ControlFlowNode for Dict | mongoengine_bad.py:57:21:57:27 | ControlFlowNode for request | mongoengine_bad.py:61:29:61:49 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | mongoengine_bad.py:61:29:61:49 | ControlFlowNode for Dict | This | mongoengine_bad.py:57:21:57:27 | ControlFlowNode for request | user-provided value |
124-
| pymongo_bad.py:14:42:14:62 | ControlFlowNode for Dict | pymongo_bad.py:11:21:11:27 | ControlFlowNode for request | pymongo_bad.py:14:42:14:62 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | pymongo_bad.py:14:42:14:62 | ControlFlowNode for Dict | This | pymongo_bad.py:11:21:11:27 | ControlFlowNode for request | user-provided value |
142+
| pymongo_test.py:15:42:15:62 | ControlFlowNode for Dict | pymongo_test.py:12:21:12:27 | ControlFlowNode for request | pymongo_test.py:15:42:15:62 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | pymongo_test.py:15:42:15:62 | ControlFlowNode for Dict | This | pymongo_test.py:12:21:12:27 | ControlFlowNode for request | user-provided value |
143+
| pymongo_test.py:33:34:33:73 | ControlFlowNode for Dict | pymongo_test.py:29:27:29:33 | ControlFlowNode for request | pymongo_test.py:33:34:33:73 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | pymongo_test.py:33:34:33:73 | ControlFlowNode for Dict | This | pymongo_test.py:29:27:29:33 | ControlFlowNode for request | user-provided value |
144+
| pymongo_test.py:43:34:43:73 | ControlFlowNode for Dict | pymongo_test.py:39:27:39:33 | ControlFlowNode for request | pymongo_test.py:43:34:43:73 | ControlFlowNode for Dict | $@ NoSQL query contains an unsanitized $@ | pymongo_test.py:43:34:43:73 | ControlFlowNode for Dict | This | pymongo_test.py:39:27:39:33 | ControlFlowNode for request | user-provided value |

‎python/ql/test/experimental/query-tests/Security/CWE-943/pymongo_bad.py

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎python/ql/test/experimental/query-tests/Security/CWE-943/pymongo_good.py

Lines changed: 0 additions & 19 deletions
This file was deleted.

‎python/ql/test/experimental/query-tests/Security/CWE-943/pymongo_test.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
from flask import Flask, request
2+
from pymongo import MongoClient
3+
from mongosanitizer.sanitizer import sanitize
4+
import json
5+
6+
app = Flask(__name__)
7+
client = MongoClient()
8+
9+
10+
@app.route("/bad")
11+
def bad():
12+
unsafe_search = request.args['search']
13+
json_search = json.loads(unsafe_search)
14+
15+
return client.db.collection.find_one({'data': json_search})
16+
17+
18+
@app.route("/good")
19+
def good():
20+
unsafe_search = request.args['search']
21+
json_search = json.loads(unsafe_search)
22+
safe_search = sanitize(json_search)
23+
24+
return client.db.collection.find_one({'data': safe_search})
25+
26+
27+
@app.route("/bad2")
28+
def bad2():
29+
event_id = json.loads(request.args['event_id'])
30+
client = MongoClient("localhost", 27017, maxPoolSize=50)
31+
db = client.localhost
32+
collection = db['collection']
33+
cursor = collection.find_one({"$where": f"this._id == '${event_id}'"})
34+
35+
36+
@app.route("/bad3")
37+
def bad3():
38+
# using `get_` methods instead of subscript/attribute lookups
39+
event_id = json.loads(request.args['event_id'])
40+
client = MongoClient("localhost", 27017, maxPoolSize=50)
41+
db = client.get_database(name="localhost")
42+
collection = db.get_collection("collection")
43+
cursor = collection.find_one({"$where": f"this._id == '${event_id}'"})
44+
45+
46+
if __name__ == "__main__":
47+
app.run(debug=True)

0 commit comments

Comments
 (0)