Python: Fix problem with missing use-use flow

python/ql/src/experimental/Security-new-dataflow/CWE-078/CommandInjection.ql

@@ -27,19 +27,30 @@ class CommandInjectionConfiguration extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override predicate isSink(DataFlow::Node sink) {
-    sink = any(SystemCommandExecution e).getCommand()
+    sink = any(SystemCommandExecution e).getCommand() and
+    // Since the implementation of os.popen looks like
+    // ```py
+    // def popen(cmd, mode="r", buffering=-1):
+    //     ...
+    //     proc = subprocess.Popen(cmd, ...)
+    // ```
+    // any time we would report flow to the `os.popen` sink, we can ALSO report the flow
+    // from the `cmd` parameter to the `subprocess.Popen` sink -- obviously we don't want
+    // that.
+    //
+    // However, simply removing taint edges out of a sink is not a good enough solution,
+    // since we would only flag one of the `os.system` calls in the following example due
+    // to use-use flow
+    // ```py
+    // os.system(cmd)
+    // os.system(cmd)
+    // ```
+    //
+    // Best solution I could come up with is to exclude all sinks inside the standard
+    // library -- this does have a downside: If we have overlooked a function in the
+    // standard library that internally runs a command, we no longer give an alert :|
+    not sink.getLocation().getFile().inStdlib()
   }
-
-  // Since the implementation of os.popen looks like
-  // ```py
-  // def popen(cmd, mode="r", buffering=-1):
-  //     ...
-  //     proc = subprocess.Popen(cmd, ...)
-  // ```
-  // any time we would report flow to the `os.popen` sink, we can ALSO report the flow
-  // from the `cmd` parameter to the `subprocess.Popen` sink -- obviously we don't want
-  // that, so to prevent that we remove any taint edges out of a sink.
-  override predicate isSanitizerOut(DataFlow::Node node) { isSink(node) }
 }
 
 from CommandInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink

python/ql/test/experimental/query-tests/Security-new-dataflow/CWE-078/CommandInjection.expected

@@ -4,7 +4,12 @@ edges
 | command_injection.py:24:11:24:22 | ControlFlowNode for Attribute | command_injection.py:25:23:25:25 | ControlFlowNode for cmd |
 | command_injection.py:30:13:30:24 | ControlFlowNode for Attribute | command_injection.py:32:14:32:26 | ControlFlowNode for BinaryExpr |
 | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | command_injection.py:39:15:39:21 | ControlFlowNode for command |
+| command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | command_injection.py:40:15:40:21 | ControlFlowNode for command |
 | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:53:15:53:21 | ControlFlowNode for command |
+| command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:54:14:54:20 | ControlFlowNode for command |
+| command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:55:21:55:27 | ControlFlowNode for command |
+| command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:56:27:56:33 | ControlFlowNode for command |
+| command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:57:20:57:26 | ControlFlowNode for command |
 nodes
 | command_injection.py:10:13:10:24 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | command_injection.py:12:15:12:27 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
@@ -16,12 +21,22 @@ nodes
 | command_injection.py:32:14:32:26 | ControlFlowNode for BinaryExpr | semmle.label | ControlFlowNode for BinaryExpr |
 | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | command_injection.py:39:15:39:21 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
+| command_injection.py:40:15:40:21 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
 | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
 | command_injection.py:53:15:53:21 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
+| command_injection.py:54:14:54:20 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
+| command_injection.py:55:21:55:27 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
+| command_injection.py:56:27:56:33 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
+| command_injection.py:57:20:57:26 | ControlFlowNode for command | semmle.label | ControlFlowNode for command |
 #select
 | command_injection.py:12:15:12:27 | ControlFlowNode for BinaryExpr | command_injection.py:10:13:10:24 | ControlFlowNode for Attribute | command_injection.py:12:15:12:27 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:10:13:10:24 | ControlFlowNode for Attribute | a user-provided value |
 | command_injection.py:19:22:19:34 | ControlFlowNode for BinaryExpr | command_injection.py:17:13:17:24 | ControlFlowNode for Attribute | command_injection.py:19:22:19:34 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:17:13:17:24 | ControlFlowNode for Attribute | a user-provided value |
 | command_injection.py:25:23:25:25 | ControlFlowNode for cmd | command_injection.py:24:11:24:22 | ControlFlowNode for Attribute | command_injection.py:25:23:25:25 | ControlFlowNode for cmd | This command depends on $@. | command_injection.py:24:11:24:22 | ControlFlowNode for Attribute | a user-provided value |
 | command_injection.py:32:14:32:26 | ControlFlowNode for BinaryExpr | command_injection.py:30:13:30:24 | ControlFlowNode for Attribute | command_injection.py:32:14:32:26 | ControlFlowNode for BinaryExpr | This command depends on $@. | command_injection.py:30:13:30:24 | ControlFlowNode for Attribute | a user-provided value |
 | command_injection.py:39:15:39:21 | ControlFlowNode for command | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | command_injection.py:39:15:39:21 | ControlFlowNode for command | This command depends on $@. | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | a user-provided value |
+| command_injection.py:40:15:40:21 | ControlFlowNode for command | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | command_injection.py:40:15:40:21 | ControlFlowNode for command | This command depends on $@. | command_injection.py:36:15:36:26 | ControlFlowNode for Attribute | a user-provided value |
 | command_injection.py:53:15:53:21 | ControlFlowNode for command | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:53:15:53:21 | ControlFlowNode for command | This command depends on $@. | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | a user-provided value |
+| command_injection.py:54:14:54:20 | ControlFlowNode for command | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:54:14:54:20 | ControlFlowNode for command | This command depends on $@. | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | a user-provided value |
+| command_injection.py:55:21:55:27 | ControlFlowNode for command | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:55:21:55:27 | ControlFlowNode for command | This command depends on $@. | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | a user-provided value |
+| command_injection.py:56:27:56:33 | ControlFlowNode for command | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:56:27:56:33 | ControlFlowNode for command | This command depends on $@. | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | a user-provided value |
+| command_injection.py:57:20:57:26 | ControlFlowNode for command | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | command_injection.py:57:20:57:26 | ControlFlowNode for command | This command depends on $@. | command_injection.py:52:15:52:26 | ControlFlowNode for Attribute | a user-provided value |