Skip to content

Commit 76c9b8c

Browse files
committed
Python: Expose importNode instead of importModule/importMember
Since predicate name `import` is not allowed, I adopted `importNode` as it sort of matches what `exprNode` does. --- Due to only using `importMember` in `os_attr` we previously didn't handle `import os.path as alias` :| I did creat a hotfix for this (#4446), but in doing so I realized the core of the problem: We're exposing ourselves to making these kinds of mistakes by having BOTH importModule and importMember, and we don't really gain anything from doing this! We do loose the ability to easily only modeling `from mod import val` and not `import mod.val`, but I don't think that will ever be relevant. This change will also make us to recognize some invalid code, for example in import os.system as runtime_error we would now model that `runtime_error` is a reference to the `os.system` function (although the actual import would result in a runtime error). Overall these are tradeoffs I'm willing to make, as it does makes things simpler from a QL modeling point of view, and THAT sounds nice 👍
1 parent 4bfd55f commit 76c9b8c

File tree

7 files changed

+13
-43
lines changed

7 files changed

+13
-43
lines changed

python/ql/src/experimental/dataflow/internal/DataFlowUtil.qll

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,8 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
3636
* Example: If `mypkg/__init__.py` contains `foo = 42`, then `from mypkg import foo` will not import the module
3737
* `mypkg/foo.py` but the variable `foo` containing `42` -- however, `import mypkg.foo` will always cause `mypkg.foo`
3838
* to refer to the module.
39-
*
40-
* Also see `DataFlow::importMember`
4139
*/
42-
Node importModule(string name) {
40+
Node importNode(string name) {
4341
exists(Variable var, Import imp, Alias alias |
4442
alias = imp.getAName() and
4543
alias.getAsname() = var.getAStore() and
@@ -72,20 +70,3 @@ Node importModule(string name) {
7270
// reference to `foo.bar`, as desired.
7371
result.asCfgNode().getNode() = any(ImportExpr i | i.getAnImportedModuleName() = name)
7472
}
75-
76-
/**
77-
* Gets a EssaNode that holds the value imported by using fully qualified name in
78-
*`from <moduleName> import <memberName>`.
79-
*
80-
* Also see `DataFlow::importModule`.
81-
*/
82-
EssaNode importMember(string moduleName, string memberName) {
83-
exists(Variable var, Import imp, Alias alias, ImportMember member |
84-
alias = imp.getAName() and
85-
member = alias.getValue() and
86-
moduleName = member.getModule().(ImportExpr).getImportedModuleName() and
87-
memberName = member.getName() and
88-
alias.getAsname() = var.getAStore() and
89-
result.getVar().(AssignmentDefinition).getSourceVariable() = var
90-
)
91-
}

python/ql/src/experimental/semmle/python/frameworks/Flask.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ private module Flask {
1515
/** Gets a reference to the `flask` module. */
1616
DataFlow::Node flask(DataFlow::TypeTracker t) {
1717
t.start() and
18-
result = DataFlow::importModule("flask")
18+
result = DataFlow::importNode("flask")
1919
or
2020
exists(DataFlow::TypeTracker t2 | result = flask(t2).track(t2, t))
2121
}
@@ -27,7 +27,7 @@ private module Flask {
2727
/** Gets a reference to the `flask.request` object. */
2828
DataFlow::Node request(DataFlow::TypeTracker t) {
2929
t.start() and
30-
result = DataFlow::importMember("flask", "request")
30+
result = DataFlow::importNode("flask.request")
3131
or
3232
t.startInAttr("request") and
3333
result = flask()

python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private module Stdlib {
1717
/** Gets a reference to the `os` module. */
1818
private DataFlow::Node os(DataFlow::TypeTracker t) {
1919
t.start() and
20-
result = DataFlow::importModule("os")
20+
result = DataFlow::importNode("os")
2121
or
2222
exists(DataFlow::TypeTracker t2 | result = os(t2).track(t2, t))
2323
}
@@ -42,10 +42,10 @@ private module Stdlib {
4242
"path"] and
4343
(
4444
t.start() and
45-
result = DataFlow::importMember("os", attr_name)
45+
result = DataFlow::importNode("os." + attr_name)
4646
or
4747
t.startInAttr(attr_name) and
48-
result = DataFlow::importModule("os")
48+
result = DataFlow::importNode("os")
4949
)
5050
or
5151
// Due to bad performance when using normal setup with `os_attr(t2, attr_name).track(t2, t)`
@@ -85,7 +85,7 @@ private module Stdlib {
8585
/** Gets a reference to the `os.path.join` function. */
8686
private DataFlow::Node join(DataFlow::TypeTracker t) {
8787
t.start() and
88-
result = DataFlow::importMember("os.path", "join")
88+
result = DataFlow::importNode("os.path.join")
8989
or
9090
t.startInAttr("join") and
9191
result = os::path()
@@ -190,7 +190,7 @@ private module Stdlib {
190190
/** Gets a reference to the `subprocess` module. */
191191
private DataFlow::Node subprocess(DataFlow::TypeTracker t) {
192192
t.start() and
193-
result = DataFlow::importModule("subprocess")
193+
result = DataFlow::importNode("subprocess")
194194
or
195195
exists(DataFlow::TypeTracker t2 | result = subprocess(t2).track(t2, t))
196196
}
@@ -208,10 +208,10 @@ private module Stdlib {
208208
attr_name in ["Popen", "call", "check_call", "check_output", "run"] and
209209
(
210210
t.start() and
211-
result = DataFlow::importMember("subprocess", attr_name)
211+
result = DataFlow::importNode("subprocess." + attr_name)
212212
or
213213
t.startInAttr(attr_name) and
214-
result = DataFlow::importModule("subprocess")
214+
result = DataFlow::importNode("subprocess")
215215
)
216216
or
217217
// Due to bad performance when using normal setup with `subprocess_attr(t2, attr_name).track(t2, t)`

python/ql/test/experimental/dataflow/import-helper/ImportHelper.expected

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
importModule
21
| test1.py:1:8:1:12 | ControlFlowNode for ImportExpr | mypkg |
32
| test1.py:1:8:1:12 | GSSA Variable mypkg | mypkg |
43
| test2.py:1:6:1:10 | ControlFlowNode for ImportExpr | mypkg |
@@ -32,9 +31,3 @@ importModule
3231
| test7.py:5:8:5:16 | GSSA Variable mypkg | mypkg |
3332
| test7.py:9:6:9:10 | ControlFlowNode for ImportExpr | mypkg |
3433
| test7.py:9:19:9:21 | GSSA Variable foo | mypkg.foo |
35-
importMember
36-
| test2.py:1:19:1:21 | GSSA Variable foo | mypkg | foo |
37-
| test2.py:1:24:1:26 | GSSA Variable bar | mypkg | bar |
38-
| test5.py:9:26:9:29 | GSSA Variable _bar | mypkg | bar |
39-
| test7.py:1:19:1:21 | GSSA Variable foo | mypkg | foo |
40-
| test7.py:9:19:9:21 | GSSA Variable foo | mypkg | foo |
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
11
import python
22
import experimental.dataflow.DataFlow
33

4-
query predicate importModule(DataFlow::Node res, string name) { res = DataFlow::importModule(name) }
5-
6-
query predicate importMember(DataFlow::Node res, string moduleName, string memberName) {
7-
res = DataFlow::importMember(moduleName, memberName)
8-
}
4+
query predicate importNode(DataFlow::Node res, string name) { res = DataFlow::importNode(name) }

python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/TestTaint.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@
140140
| test_string.py:159 | ok | test_os_path_join | os.path.join(..) |
141141
| test_string.py:160 | ok | test_os_path_join | os.path.join(..) |
142142
| test_string.py:161 | ok | test_os_path_join | os.path.join(..) |
143-
| test_string.py:162 | fail | test_os_path_join | ospath_alias.join(..) |
143+
| test_string.py:162 | ok | test_os_path_join | ospath_alias.join(..) |
144144
| test_unpacking.py:16 | ok | unpacking | a |
145145
| test_unpacking.py:16 | ok | unpacking | b |
146146
| test_unpacking.py:16 | ok | unpacking | c |

python/ql/test/experimental/dataflow/typetracking/moduleattr.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import experimental.dataflow.TypeTracker
44

55
DataFlow::Node module_tracker(TypeTracker t) {
66
t.start() and
7-
result = DataFlow::importModule("module")
7+
result = DataFlow::importNode("module")
88
or
99
exists(TypeTracker t2 | result = module_tracker(t2).track(t2, t))
1010
}

0 commit comments

Comments
 (0)