Skip to content

Commit baafd9f

Browse files
RasmusWLRasmusWL
committed
Python: Add an other path injection FP
Along with the root cause, which is the `StringConstCompare` BarrierGuard, that does only allows `in <iterable literal>` and not `in <variable referencing iterable literal>`
1 parent a68b55b commit baafd9f

File tree

3 files changed

+71
-0
lines changed

3 files changed

+71
-0
lines changed

‎python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/test_string_const_compare.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,57 @@ def test_in_set():
7777
ensure_tainted(ts) # $ tainted
7878

7979

80+
def test_in_local_variable():
81+
ts = TAINTED_STRING
82+
safe = ["safe", "also_safe"]
83+
if ts in safe:
84+
ensure_not_tainted(ts) # $ SPURIOUS: tainted
85+
else:
86+
ensure_tainted(ts) # $ tainted
87+
88+
89+
SAFE = ["safe", "also_safe"]
90+
91+
92+
def test_in_global_variable():
93+
ts = TAINTED_STRING
94+
if ts in SAFE:
95+
ensure_not_tainted(ts) # $ SPURIOUS: tainted
96+
else:
97+
ensure_tainted(ts) # $ tainted
98+
99+
100+
# these global variables can be modified, so should not be considered safe
101+
SAFE_mod_1 = ["safe", "also_safe"]
102+
SAFE_mod_2 = ["safe", "also_safe"]
103+
SAFE_mod_3 = ["safe", "also_safe"]
104+
105+
106+
def make_modification(x):
107+
global SAFE_mod_2, SAFE_mod_3
108+
SAFE_mod_1.append(x)
109+
SAFE_mod_2 += [x]
110+
SAFE_mod_3 = SAFE_mod_3 + [x]
111+
112+
113+
def test_in_modified_global_variable():
114+
ts = TAINTED_STRING
115+
if ts in SAFE_mod_1:
116+
ensure_tainted(ts) # $ tainted
117+
else:
118+
ensure_tainted(ts) # $ tainted
119+
120+
if ts in SAFE_mod_2:
121+
ensure_tainted(ts) # $ tainted
122+
else:
123+
ensure_tainted(ts) # $ tainted
124+
125+
if ts in SAFE_mod_3:
126+
ensure_tainted(ts) # $ tainted
127+
else:
128+
ensure_tainted(ts) # $ tainted
129+
130+
80131
def test_in_unsafe1(xs):
81132
ts = TAINTED_STRING
82133
if ts in xs:
@@ -131,6 +182,10 @@ def test_eq_thorugh_func():
131182
test_in_list()
132183
test_in_tuple()
133184
test_in_set()
185+
test_in_local_variable()
186+
test_in_global_variable()
187+
make_modification("unsafe")
188+
test_in_modified_global_variable()
134189
test_in_unsafe1(["unsafe", "foo"])
135190
test_in_unsafe2("unsafe")
136191
test_not_in1()

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ edges
3737
| path_injection.py:138:16:138:22 | ControlFlowNode for request | path_injection.py:138:16:138:27 | ControlFlowNode for Attribute |
3838
| path_injection.py:138:16:138:27 | ControlFlowNode for Attribute | path_injection.py:140:30:140:51 | ControlFlowNode for Attribute() |
3939
| path_injection.py:138:16:138:27 | ControlFlowNode for Attribute | path_injection.py:142:14:142:17 | ControlFlowNode for path |
40+
| path_injection.py:149:16:149:22 | ControlFlowNode for request | path_injection.py:149:16:149:27 | ControlFlowNode for Attribute |
41+
| path_injection.py:149:16:149:27 | ControlFlowNode for Attribute | path_injection.py:152:18:152:21 | ControlFlowNode for path |
4042
| test.py:9:12:9:18 | ControlFlowNode for request | test.py:9:12:9:23 | ControlFlowNode for Attribute |
4143
| test.py:9:12:9:18 | ControlFlowNode for request | test.py:9:12:9:23 | ControlFlowNode for Attribute |
4244
| test.py:9:12:9:23 | ControlFlowNode for Attribute | test.py:9:12:9:39 | ControlFlowNode for Attribute() |
@@ -130,6 +132,9 @@ nodes
130132
| path_injection.py:138:16:138:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
131133
| path_injection.py:140:30:140:51 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
132134
| path_injection.py:142:14:142:17 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
135+
| path_injection.py:149:16:149:22 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
136+
| path_injection.py:149:16:149:27 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
137+
| path_injection.py:152:18:152:21 | ControlFlowNode for path | semmle.label | ControlFlowNode for path |
133138
| test.py:9:12:9:18 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
134139
| test.py:9:12:9:18 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
135140
| test.py:9:12:9:23 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
@@ -181,6 +186,7 @@ nodes
181186
| path_injection.py:124:14:124:17 | ControlFlowNode for path | path_injection.py:118:16:118:22 | ControlFlowNode for request | path_injection.py:124:14:124:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:118:16:118:22 | ControlFlowNode for request | a user-provided value |
182187
| path_injection.py:132:14:132:22 | ControlFlowNode for sanitized | path_injection.py:129:16:129:22 | ControlFlowNode for request | path_injection.py:132:14:132:22 | ControlFlowNode for sanitized | This path depends on $@. | path_injection.py:129:16:129:22 | ControlFlowNode for request | a user-provided value |
183188
| path_injection.py:142:14:142:17 | ControlFlowNode for path | path_injection.py:138:16:138:22 | ControlFlowNode for request | path_injection.py:142:14:142:17 | ControlFlowNode for path | This path depends on $@. | path_injection.py:138:16:138:22 | ControlFlowNode for request | a user-provided value |
189+
| path_injection.py:152:18:152:21 | ControlFlowNode for path | path_injection.py:149:16:149:22 | ControlFlowNode for request | path_injection.py:152:18:152:21 | ControlFlowNode for path | This path depends on $@. | path_injection.py:149:16:149:22 | ControlFlowNode for request | a user-provided value |
184190
| test.py:19:10:19:10 | ControlFlowNode for x | test.py:9:12:9:18 | ControlFlowNode for request | test.py:19:10:19:10 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:18 | ControlFlowNode for request | a user-provided value |
185191
| test.py:26:10:26:10 | ControlFlowNode for y | test.py:9:12:9:18 | ControlFlowNode for request | test.py:26:10:26:10 | ControlFlowNode for y | This path depends on $@. | test.py:9:12:9:18 | ControlFlowNode for request | a user-provided value |
186192
| test.py:33:14:33:14 | ControlFlowNode for x | test.py:9:12:9:18 | ControlFlowNode for request | test.py:33:14:33:14 | ControlFlowNode for x | This path depends on $@. | test.py:9:12:9:18 | ControlFlowNode for request | a user-provided value |

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,13 @@ def stackoverflow_solution():
140140
if os.path.commonprefix((os.path.realpath(path), STATIC_DIR)) != STATIC_DIR:
141141
return "not this time"
142142
f = open(path) # OK TODO: FP
143+
144+
145+
SAFE_FILES = ['foo', 'bar', 'baz']
146+
147+
@app.route("/safe-set-of-files")
148+
def safe_set_of_files():
149+
filename = request.args.get('filename', '')
150+
if filename in SAFE_FILES:
151+
path = os.path.join(STATIC_DIR, filename)
152+
f = open(path) # OK TODO: FP

0 commit comments

Comments
 (0)