Skip to content

Commit a9ce067

Browse files
RasmusWLRasmusWL
committed
Python: Add examples of Path Injection FPs seen
Not quite sure how to deal with these cases of safe if UNIX-only, otherwise not safe. If/when we actually try to deal with these, we also need to figure that out. We _could_ split this queyr into 3: (1) for path injection on any platform, (2) path injection on windows, (3) path injection on UNIX. Then UNIX-only projects could disable the path-injection on windows query. -- that's my best idea, if you have better ideas, DO tell 👍
1 parent e8f6331 commit a9ce067

File tree

2 files changed

+82
-0
lines changed

2 files changed

+82
-0
lines changed

‎python/ql/test/query-tests/Security/CWE-022-PathInjection/PathInjection.expected‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ edges
1111
| path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | path_injection.py:64:13:64:63 | ControlFlowNode for Attribute() |
1212
| path_injection.py:64:13:64:63 | ControlFlowNode for Attribute() | path_injection.py:65:14:65:18 | ControlFlowNode for npath |
1313
| path_injection.py:71:16:71:27 | ControlFlowNode for Attribute | path_injection.py:72:13:72:63 | ControlFlowNode for Attribute() |
14+
| path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | path_injection.py:81:14:81:17 | ControlFlowNode for path |
15+
| path_injection.py:85:20:85:22 | ControlFlowNode for foo | path_injection.py:89:14:89:17 | ControlFlowNode for path |
16+
| path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | path_injection.py:100:14:100:17 | ControlFlowNode for path |
17+
| path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | path_injection.py:111:14:111:17 | ControlFlowNode for path |
18+
| path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | path_injection.py:119:14:119:22 | ControlFlowNode for sanitized |
19+
| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:127:30:127:51 | ControlFlowNode for Attribute() |
20+
| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:129:14:129:17 | ControlFlowNode for path |
1421
| test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
1522
| test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
1623
| test.py:9:12:9:39 | ControlFlowNode for Attribute() | test.py:18:9:18:16 | ControlFlowNode for source() |
@@ -65,6 +72,20 @@ nodes
6572
| path_injection.py:65:14:65:18 | ControlFlowNode for npath | semmle.label | ControlFlowNode for npath |
6673
| path_injection.py:71:16:71:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
6774
| path_injection.py:72:13:72:63 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
75+
| path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | semmle.label | ControlFlowNode for foo_id |
76+
| path_injection.py:81:14:81:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
77+
| path_injection.py:85:20:85:22 | ControlFlowNode for foo | semmle.label | ControlFlowNode for foo |
78+
| path_injection.py:89:14:89:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
79+
| path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
80+
| path_injection.py:100:14:100:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
81+
| path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
82+
| path_injection.py:111:14:111:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
83+
| path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
84+
| path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | semmle.label | ControlFlowNode for sanitized |
85+
| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
86+
| path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
87+
| path_injection.py:127:30:127:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
88+
| path_injection.py:129:14:129:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
6889
| test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
6990
| test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
7091
| test.py:9:12:9:39 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
@@ -105,6 +126,12 @@ nodes
105126
| path_injection.py:31:14:31:18 | ControlFlowNode for npath | path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | path_injection.py:31:14:31:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:27:16:27:27 | ControlFlowNode for Attribute | a user-provided value |
106127
| path_injection.py:48:14:48:18 | ControlFlowNode for npath | path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | path_injection.py:48:14:48:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:46:16:46:27 | ControlFlowNode for Attribute | a user-provided value |
107128
| path_injection.py:65:14:65:18 | ControlFlowNode for npath | path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | path_injection.py:65:14:65:18 | ControlFlowNode for npath | This path depends on $@. | path_injection.py:63:16:63:27 | ControlFlowNode for Attribute | a user-provided value |
129+
| path_injection.py:81:14:81:17 | ControlFlowNode for path | path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | path_injection.py:81:14:81:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:78:20:78:25 | ControlFlowNode for foo_id | a user-provided value |
130+
| path_injection.py:89:14:89:17 | ControlFlowNode for path | path_injection.py:85:20:85:22 | ControlFlowNode for foo | path_injection.py:89:14:89:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:85:20:85:22 | ControlFlowNode for foo | a user-provided value |
131+
| path_injection.py:100:14:100:17 | ControlFlowNode for path | path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | path_injection.py:100:14:100:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:94:16:94:27 | ControlFlowNode for Attribute | a user-provided value |
132+
| path_injection.py:111:14:111:17 | ControlFlowNode for path | path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | path_injection.py:111:14:111:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:105:16:105:27 | ControlFlowNode for Attribute | a user-provided value |
133+
| path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | path_injection.py:119:14:119:22 | ControlFlowNode for sanitized | This path depends on $@. | path_injection.py:116:16:116:27 | ControlFlowNode for Attribute | a user-provided value |
134+
| path_injection.py:129:14:129:17 | ControlFlowNode for path | path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | path_injection.py:129:14:129:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:125:16:125:27 | ControlFlowNode for Attribute | a user-provided value |
108135
| test.py:19:10:19:10 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:19:10:19:10 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
109136
| test.py:26:10:26:10 | ControlFlowNode for y | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:26:10:26:10 | ControlFlowNode for y | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |
110137
| test.py:33:14:33:14 | ControlFlowNode for x | test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:33:14:33:14 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:23 | ControlFlowNode for Attribute | a user-provided value |

‎python/ql/test/query-tests/Security/CWE-022-PathInjection/path_injection.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,58 @@ def safe_path_abspath():
7272
npath = os.path.abspath(os.path.join(STATIC_DIR, filename))
7373
if npath.startswith(STATIC_DIR):
7474
f = open(npath) # OK
75+
76+
77+
@app.route("/int-only/<int:foo_id>")
78+
def flask_int_only(foo_id):
79+
# This is OK, since the flask routing ensures that `foo_id` MUST be an integer.
80+
path = os.path.join(STATIC_DIR, foo_id)
81+
f = open(path) # OK TODO: FP
82+
83+
84+
@app.route("/not-path/<foo>")
85+
def flask_not_path(foo):
86+
# On UNIX systems, this is OK, since without being marked as `<path:foo>`, flask
87+
# routing ensures that `foo` cannot contain forward slashes (not by using %2F either).
88+
path = os.path.join(STATIC_DIR, foo)
89+
f = open(path) # OK if only running on UNIX systems, NOT OK if could be running on windows
90+
91+
92+
@app.route("/no-dot-dot")
93+
def no_dot_dot():
94+
filename = request.args.get('filename', '')
95+
path = os.path.join(STATIC_DIR, filename)
96+
# Note: even for UNIX-only programs, this check is not good enough, since it doesn't
97+
# handle if `filename` is an absolute path
98+
if '../' in path:
99+
return "not this time"
100+
f = open(path) # NOT OK
101+
102+
103+
@app.route("/no-dot-dot-with-prefix")
104+
def no_dot_dot_with_prefix():
105+
filename = request.args.get('filename', '')
106+
path = os.path.join(STATIC_DIR, "img-"+filename)
107+
# Note: Since `filename` has a prefix, it's not possible to use an absolute path.
108+
# Therefore, for UNIX-only programs, the `../` check is enough to stop path injections.
109+
if '../' in path:
110+
return "not this time"
111+
f = open(path) # OK if only running on UNIX systems, NOT OK if could be running on windows
112+
113+
114+
@app.route("/replace-slash")
115+
def replace_slash():
116+
filename = request.args.get('filename', '')
117+
path = os.path.join(STATIC_DIR, filename)
118+
sanitized = path.replace("/", "_")
119+
f = open(sanitized) # OK if only running on UNIX systems, NOT OK if could be running on windows
120+
121+
122+
@app.route("/stackoverflow-solution")
123+
def stackoverflow_solution():
124+
# Solution provided in https://stackoverflow.com/a/45188896
125+
filename = request.args.get('filename', '')
126+
path = os.path.join(STATIC_DIR, filename)
127+
if os.path.commonprefix((os.path.realpath(path), STATIC_DIR)) != STATIC_DIR:
128+
return "not this time"
129+
f = open(path) # OK TODO: FP

0 commit comments

Comments
 (0)