From 0a7cfad04892cda48d0f34bccf68269165674947 Mon Sep 17 00:00:00 2001
From: Rasmus Lerchedahl Petersen <yoff@github.com>
Date: Wed, 2 Nov 2022 16:21:59 +0100
Subject: [PATCH] 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.
---
 .../DataflowQueryTest.expected                |  2 ++
 .../DataflowQueryTest.ql                      |  3 +++
 .../command_injection.py                      | 26 +++++++++----------
 3 files changed, 18 insertions(+), 13 deletions(-)
 create mode 100644 python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.expected
 create mode 100644 python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.ql

diff --git a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.expected b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.expected
new file mode 100644
index 00000000000..3875da4e143
--- /dev/null
+++ b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.expected
@@ -0,0 +1,2 @@
+missingAnnotationOnSink
+failures
diff --git a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.ql b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.ql
new file mode 100644
index 00000000000..c69cc5c7578
--- /dev/null
+++ b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/DataflowQueryTest.ql
@@ -0,0 +1,3 @@
+import python
+import experimental.dataflow.TestUtil.DataflowQueryTest
+import semmle.python.security.dataflow.CommandInjectionQuery
diff --git a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py
index 978a2bf986c..ef8cb221ea0 100644
--- a/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py
+++ b/python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py
@@ -10,27 +10,27 @@ app = Flask(__name__)
 def command_injection1():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    os.system("ls " + files)
+    os.system("ls " + files) # $flow="ImportMember, l:-8 -> BinaryExpr"
 
 
 @app.route("/command2")
 def command_injection2():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    subprocess.Popen("ls " + files, shell=True)
+    subprocess.Popen("ls " + files, shell=True) # $flow="ImportMember, l:-15 -> BinaryExpr"
 
 
 @app.route("/command3")
 def first_arg_injection():
     cmd = request.args.get('cmd', '')
-    subprocess.Popen([cmd, "param1"])
+    subprocess.Popen([cmd, "param1"]) # $flow="ImportMember, l:-21 -> cmd"
 
 
 @app.route("/other_cases")
 def others():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    os.popen("ls " + files)
+    os.popen("ls " + files) # $flow="ImportMember, l:-28 -> BinaryExpr"
 
 
 @app.route("/multiple")
@@ -38,8 +38,8 @@ def multiple():
     command = request.args.get('command', '')
     # We should mark flow to both calls here, which conflicts with removing flow out of
     # a sink due to use-use flow.
-    os.system(command)
-    os.system(command)
+    os.system(command) # $flow="ImportMember, l:-36 -> command"
+    os.system(command) # $flow="ImportMember, l:-37 -> command"
 
 
 @app.route("/not-into-sink-impl")
@@ -52,11 +52,11 @@ def not_into_sink_impl():
     subprocess.call implementation: https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
     """
     command = request.args.get('command', '')
-    os.system(command)
-    os.popen(command)
-    subprocess.call(command)
-    subprocess.check_call(command)
-    subprocess.run(command)
+    os.system(command) # $flow="ImportMember, l:-50 -> command"
+    os.popen(command) # $flow="ImportMember, l:-51 -> command"
+    subprocess.call(command) # $flow="ImportMember, l:-52 -> command"
+    subprocess.check_call(command) # $flow="ImportMember, l:-53 -> command"
+    subprocess.run(command) # $flow="ImportMember, l:-54 -> command"
 
 
 @app.route("/path-exists-not-sanitizer")
@@ -70,11 +70,11 @@ def path_exists_not_sanitizer():
     """
     path = request.args.get('path', '')
     if os.path.exists(path):
-        os.system("ls " + path) # NOT OK
+        os.system("ls " + path) # $flow="ImportMember, l:-68 -> BinaryExpr"
 
 
 @app.route("/restricted-characters")
 def restricted_characters():
     path = request.args.get('path', '')
     if re.match(r'^[a-zA-Z0-9_-]+$', path):
-        os.system("ls " + path) # OK (TODO: Currently FP)
+        os.system("ls " + path) # $SPURIOUS: flow="ImportMember, l:-75 -> BinaryExpr"