python: fix implementation of lambdaCreation

- still identifying summarized callables by name.
I think ther shoudl perhaps be a `getAUse` next to `getACall`.
- also fix tests, adding a standard taint configuration

Author: yoff

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

@@ -936,14 +936,16 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c)
   creation.asExpr() = c.(DataFlowLambda).getDefinition()
   or
   // normal function
-  // TODO: reconsider this code
-  kind = kind and
-  exists(Call call, Name f, FunctionDef def |
-    f = call.getAnArg() and
-    def.getDefinedFunction().getName() = f.getId() and
-    // c.getCallableValue() = def.getDefinedFunction().getDefinition() and
-    c.getName() = f.getId() and
-    creation.asExpr() = call
+  exists(FunctionDef def |
+    def.defines(creation.asVar().getSourceVariable()) and
+    def.getDefinedFunction() = c.(DataFlowCallableValue).getCallableValue().getScope()
+  )
+  or
+  // summarized function
+  exists(Call call, Name arg |
+    arg = call.getAnArg() and
+    c.(LibraryCallableValue).getName() = arg.getId() and
+    creation.asExpr() = arg
   )
 }
 

python/ql/test/experimental/dataflow/TestUtil/NormalTaintTrackingTest.qll

@@ -0,0 +1,33 @@
+import python
+import experimental.dataflow.TestUtil.FlowTest
+import experimental.dataflow.testTaintConfig
+private import semmle.python.dataflow.new.internal.PrintNode
+
+class DataFlowTest extends FlowTest {
+  DataFlowTest() { this = "DataFlowTest" }
+
+  override string flowTag() { result = "flow" }
+
+  override predicate relevantFlow(DataFlow::Node source, DataFlow::Node sink) {
+    exists(TestConfiguration cfg | cfg.hasFlow(source, sink))
+  }
+}
+
+query predicate missingAnnotationOnSINK(Location location, string error, string element) {
+  error = "ERROR, you should add `# $ MISSING: flow` annotation" and
+  exists(DataFlow::Node sink |
+    exists(DataFlow::CallCfgNode call |
+      // note: we only care about `SINK` and not `SINK_F`, so we have to reconstruct manually.
+      call.getFunction().asCfgNode().(NameNode).getId() = "SINK" and
+      (sink = call.getArg(_) or sink = call.getArgByName(_))
+    ) and
+    location = sink.getLocation() and
+    element = prettyExpr(sink.asExpr()) and
+    not any(TestConfiguration config).hasFlow(_, sink) and
+    not exists(FalseNegativeExpectation missingResult |
+      missingResult.getTag() = "flow" and
+      missingResult.getLocation().getFile() = location.getFile() and
+      missingResult.getLocation().getStartLine() = location.getStartLine()
+    )
+  )
+}

python/ql/test/experimental/dataflow/summaries/NormalDataflowTest.ql

@@ -1,3 +1,3 @@
 import python
 private import TestSummaries
-import experimental.dataflow.TestUtil.NormalDataflowTest
+import experimental.dataflow.TestUtil.NormalTaintTrackingTest

python/ql/test/experimental/dataflow/summaries/summaries.expected

@@ -1,20 +1,12 @@
 edges
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:32:20:32:25 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:36:48:36:53 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:44:25:44:32 | ControlFlowNode for List |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:44:26:44:31 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:51:34:51:39 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:57:51:57:56 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:60:41:60:46 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:64:33:64:38 | ControlFlowNode for SOURCE |
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:65:6:65:26 | ControlFlowNode for Subscript |
 | summaries.py:32:11:32:26 | ControlFlowNode for identity() | summaries.py:33:6:33:12 | ControlFlowNode for tainted |
 | summaries.py:32:20:32:25 | ControlFlowNode for SOURCE | summaries.py:32:11:32:26 | ControlFlowNode for identity() |
 | summaries.py:36:18:36:54 | ControlFlowNode for apply_lambda() | summaries.py:37:6:37:19 | ControlFlowNode for tainted_lambda |
 | summaries.py:36:48:36:53 | ControlFlowNode for SOURCE | summaries.py:36:18:36:54 | ControlFlowNode for apply_lambda() |
 | summaries.py:44:16:44:33 | ControlFlowNode for reversed() [List element] | summaries.py:45:6:45:17 | ControlFlowNode for tainted_list [List element] |
 | summaries.py:44:25:44:32 | ControlFlowNode for List | summaries.py:45:6:45:20 | ControlFlowNode for Subscript |
 | summaries.py:44:25:44:32 | ControlFlowNode for List [List element] | summaries.py:44:16:44:33 | ControlFlowNode for reversed() [List element] |
+| summaries.py:44:26:44:31 | ControlFlowNode for SOURCE | summaries.py:44:25:44:32 | ControlFlowNode for List |
 | summaries.py:44:26:44:31 | ControlFlowNode for SOURCE | summaries.py:44:25:44:32 | ControlFlowNode for List [List element] |
 | summaries.py:45:6:45:17 | ControlFlowNode for tainted_list [List element] | summaries.py:45:6:45:20 | ControlFlowNode for Subscript |
 | summaries.py:51:18:51:41 | ControlFlowNode for map() [List element] | summaries.py:52:6:52:19 | ControlFlowNode for tainted_mapped [List element] |
@@ -31,9 +23,9 @@ edges
 | summaries.py:61:6:61:27 | ControlFlowNode for tainted_mapped_summary [List element] | summaries.py:61:6:61:30 | ControlFlowNode for Subscript |
 | summaries.py:64:22:64:39 | ControlFlowNode for json_loads() [List element] | summaries.py:65:6:65:23 | ControlFlowNode for tainted_resultlist [List element] |
 | summaries.py:64:33:64:38 | ControlFlowNode for SOURCE | summaries.py:64:22:64:39 | ControlFlowNode for json_loads() [List element] |
+| summaries.py:64:33:64:38 | ControlFlowNode for SOURCE | summaries.py:65:6:65:26 | ControlFlowNode for Subscript |
 | summaries.py:65:6:65:23 | ControlFlowNode for tainted_resultlist [List element] | summaries.py:65:6:65:26 | ControlFlowNode for Subscript |
 nodes
-| summaries.py:10:10:10:17 | ControlFlowNode for Str | semmle.label | ControlFlowNode for Str |
 | summaries.py:32:11:32:26 | ControlFlowNode for identity() | semmle.label | ControlFlowNode for identity() |
 | summaries.py:32:20:32:25 | ControlFlowNode for SOURCE | semmle.label | ControlFlowNode for SOURCE |
 | summaries.py:33:6:33:12 | ControlFlowNode for tainted | semmle.label | ControlFlowNode for tainted |
@@ -68,10 +60,10 @@ nodes
 subpaths
 invalidSpecComponent
 #select
-| summaries.py:33:6:33:12 | ControlFlowNode for tainted | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:33:6:33:12 | ControlFlowNode for tainted | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:37:6:37:19 | ControlFlowNode for tainted_lambda | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:37:6:37:19 | ControlFlowNode for tainted_lambda | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:45:6:45:20 | ControlFlowNode for Subscript | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:45:6:45:20 | ControlFlowNode for Subscript | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:52:6:52:22 | ControlFlowNode for Subscript | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:52:6:52:22 | ControlFlowNode for Subscript | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:58:6:58:31 | ControlFlowNode for Subscript | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:58:6:58:31 | ControlFlowNode for Subscript | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:61:6:61:30 | ControlFlowNode for Subscript | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:61:6:61:30 | ControlFlowNode for Subscript | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
-| summaries.py:65:6:65:26 | ControlFlowNode for Subscript | summaries.py:10:10:10:17 | ControlFlowNode for Str | summaries.py:65:6:65:26 | ControlFlowNode for Subscript | $@ | summaries.py:10:10:10:17 | ControlFlowNode for Str | ControlFlowNode for Str |
+| summaries.py:33:6:33:12 | ControlFlowNode for tainted | summaries.py:32:20:32:25 | ControlFlowNode for SOURCE | summaries.py:33:6:33:12 | ControlFlowNode for tainted | $@ | summaries.py:32:20:32:25 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:37:6:37:19 | ControlFlowNode for tainted_lambda | summaries.py:36:48:36:53 | ControlFlowNode for SOURCE | summaries.py:37:6:37:19 | ControlFlowNode for tainted_lambda | $@ | summaries.py:36:48:36:53 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:45:6:45:20 | ControlFlowNode for Subscript | summaries.py:44:26:44:31 | ControlFlowNode for SOURCE | summaries.py:45:6:45:20 | ControlFlowNode for Subscript | $@ | summaries.py:44:26:44:31 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:52:6:52:22 | ControlFlowNode for Subscript | summaries.py:51:34:51:39 | ControlFlowNode for SOURCE | summaries.py:52:6:52:22 | ControlFlowNode for Subscript | $@ | summaries.py:51:34:51:39 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:58:6:58:31 | ControlFlowNode for Subscript | summaries.py:57:51:57:56 | ControlFlowNode for SOURCE | summaries.py:58:6:58:31 | ControlFlowNode for Subscript | $@ | summaries.py:57:51:57:56 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:61:6:61:30 | ControlFlowNode for Subscript | summaries.py:60:41:60:46 | ControlFlowNode for SOURCE | summaries.py:61:6:61:30 | ControlFlowNode for Subscript | $@ | summaries.py:60:41:60:46 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |
+| summaries.py:65:6:65:26 | ControlFlowNode for Subscript | summaries.py:64:33:64:38 | ControlFlowNode for SOURCE | summaries.py:65:6:65:26 | ControlFlowNode for Subscript | $@ | summaries.py:64:33:64:38 | ControlFlowNode for SOURCE | ControlFlowNode for SOURCE |

python/ql/test/experimental/dataflow/summaries/summaries.py

@@ -38,7 +38,7 @@ def SINK_F(x):
 
 # A lambda that breaks the flow
 untainted_lambda = apply_lambda(lambda x: 1, SOURCE)
-SINK_F(untainted_lambda) # $ SPURIOUS: flow="SOURCE, l:-1 -> untainted_lambda"
+SINK_F(untainted_lambda)
 
 # Collection summaries
 tainted_list = reversed([SOURCE])

python/ql/test/experimental/dataflow/summaries/summaries.ql

@@ -8,26 +8,14 @@ import DataFlow::PathGraph
 import semmle.python.dataflow.new.TaintTracking
 import semmle.python.dataflow.new.internal.FlowSummaryImpl
 import semmle.python.ApiGraphs
+import experimental.dataflow.testTaintConfig
 private import TestSummaries
 
 query predicate invalidSpecComponent(SummarizedCallable sc, string s, string c) {
   (sc.propagatesFlowExt(s, _, _) or sc.propagatesFlowExt(_, s, _)) and
   Private::External::invalidSpecComponent(s, c)
 }
 
-class Conf extends TaintTracking::Configuration {
-  Conf() { this = "FlowSummaries" }
-
-  override predicate isSource(DataFlow::Node src) { src.asExpr().(StrConst).getS() = "source" }
-
-  override predicate isSink(DataFlow::Node sink) {
-    exists(Call mc |
-      mc.getFunc().(Name).getId() = "SINK" and
-      mc.getAnArg() = sink.asExpr()
-    )
-  }
-}
-
-from DataFlow::PathNode source, DataFlow::PathNode sink, Conf conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, TestConfiguration conf
 where conf.hasFlowPath(source, sink)
 select sink, source, sink, "$@", source, source.toString()

python/ql/test/experimental/dataflow/testTaintConfig.qll

@@ -0,0 +1,51 @@
+/**
+ * Configuration to test selected data flow
+ * Sources in the source code are denoted by the special name `SOURCE`,
+ * and sinks are denoted by arguments to the special function `SINK`.
+ * For example, given the test code
+ * ```python
+ *  def test():
+ *      s = SOURCE
+ *      SINK(s)
+ * ```
+ * `SOURCE` will be a source and the second occurance of `s` will be a sink.
+ *
+ * In order to test literals, alternative sources are defined for each type:
+ *
+ *  for | use
+ * ----------
+ * string | `"source"`
+ * integer | `42`
+ * float | `42.0`
+ * complex | `42j` (not supported yet)
+ */
+
+private import python
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+
+class TestConfiguration extends TaintTracking::Configuration {
+  TestConfiguration() { this = "TestConfiguration" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.(DataFlow::CfgNode).getNode().(NameNode).getId() = "SOURCE"
+    or
+    node.(DataFlow::CfgNode).getNode().getNode().(StrConst).getS() = "source"
+    or
+    node.(DataFlow::CfgNode).getNode().getNode().(IntegerLiteral).getN() = "42"
+    or
+    node.(DataFlow::CfgNode).getNode().getNode().(FloatLiteral).getN() = "42.0"
+    // No support for complex numbers
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(CallNode call |
+      call.getFunction().(NameNode).getId() in ["SINK", "SINK_F"] and
+      node.(DataFlow::CfgNode).getNode() = call.getAnArg()
+    )
+  }
+
+  override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
+
+  override int explorationLimit() { result = 5 }
+}