Skip to content

Commit 0a7cfad

Browse files
committed
python: inline query tests for command injection
note how the test file is partially annotated and those annotations can now be expressed In this particular test file, absolute line numbers might have been better than relative ones. We might remove line numbers altogether, but should check more querries to see how it looks.
1 parent f486c44 commit 0a7cfad

File tree

3 files changed

+18
-13
lines changed

3 files changed

+18
-13
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
missingAnnotationOnSink
2+
failures
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import python
2+
import experimental.dataflow.TestUtil.DataflowQueryTest
3+
import semmle.python.security.dataflow.CommandInjectionQuery

python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,36 @@
1010
def command_injection1():
1111
files = request.args.get('files', '')
1212
# Don't let files be `; rm -rf /`
13-
os.system("ls " + files)
13+
os.system("ls " + files) # $flow="ImportMember, l:-8 -> BinaryExpr"
1414

1515

1616
@app.route("/command2")
1717
def command_injection2():
1818
files = request.args.get('files', '')
1919
# Don't let files be `; rm -rf /`
20-
subprocess.Popen("ls " + files, shell=True)
20+
subprocess.Popen("ls " + files, shell=True) # $flow="ImportMember, l:-15 -> BinaryExpr"
2121

2222

2323
@app.route("/command3")
2424
def first_arg_injection():
2525
cmd = request.args.get('cmd', '')
26-
subprocess.Popen([cmd, "param1"])
26+
subprocess.Popen([cmd, "param1"]) # $flow="ImportMember, l:-21 -> cmd"
2727

2828

2929
@app.route("/other_cases")
3030
def others():
3131
files = request.args.get('files', '')
3232
# Don't let files be `; rm -rf /`
33-
os.popen("ls " + files)
33+
os.popen("ls " + files) # $flow="ImportMember, l:-28 -> BinaryExpr"
3434

3535

3636
@app.route("/multiple")
3737
def multiple():
3838
command = request.args.get('command', '')
3939
# We should mark flow to both calls here, which conflicts with removing flow out of
4040
# a sink due to use-use flow.
41-
os.system(command)
42-
os.system(command)
41+
os.system(command) # $flow="ImportMember, l:-36 -> command"
42+
os.system(command) # $flow="ImportMember, l:-37 -> command"
4343

4444

4545
@app.route("/not-into-sink-impl")
@@ -52,11 +52,11 @@ def not_into_sink_impl():
5252
subprocess.call implementation: https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
5353
"""
5454
command = request.args.get('command', '')
55-
os.system(command)
56-
os.popen(command)
57-
subprocess.call(command)
58-
subprocess.check_call(command)
59-
subprocess.run(command)
55+
os.system(command) # $flow="ImportMember, l:-50 -> command"
56+
os.popen(command) # $flow="ImportMember, l:-51 -> command"
57+
subprocess.call(command) # $flow="ImportMember, l:-52 -> command"
58+
subprocess.check_call(command) # $flow="ImportMember, l:-53 -> command"
59+
subprocess.run(command) # $flow="ImportMember, l:-54 -> command"
6060

6161

6262
@app.route("/path-exists-not-sanitizer")
@@ -70,11 +70,11 @@ def path_exists_not_sanitizer():
7070
"""
7171
path = request.args.get('path', '')
7272
if os.path.exists(path):
73-
os.system("ls " + path) # NOT OK
73+
os.system("ls " + path) # $flow="ImportMember, l:-68 -> BinaryExpr"
7474

7575

7676
@app.route("/restricted-characters")
7777
def restricted_characters():
7878
path = request.args.get('path', '')
7979
if re.match(r'^[a-zA-Z0-9_-]+$', path):
80-
os.system("ls " + path) # OK (TODO: Currently FP)
80+
os.system("ls " + path) # $SPURIOUS: flow="ImportMember, l:-75 -> BinaryExpr"

0 commit comments

Comments
 (0)