Python: remove NonLibraryDataFlowCallable

this required managing parameters and their pre-update nodes a bit

Author: yoff

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

@@ -103,10 +103,9 @@ module ArgumentPassing {
    * Used to limit the size of predicates.
    */
   predicate connects(CallNode call, CallableValue callable) {
-    exists(NormalCall c, NonLibraryDataFlowCallable k |
+    exists(NormalCall c |
       call = c.getNode() and
-      callable = k.getCallableValue() and
-      k = c.getCallable()
+      callable = c.getCallable().getCallableValue()
     )
   }
 
@@ -322,31 +321,24 @@ class DataFlowCallable extends TDataFlowCallable {
   /** Gets the name of this callable. */
   string getName() { none() }
 
+  /** Gets a callable value for this callable, if any. */
+  CallableValue getCallableValue() { none() }
+
   /** Gets the underlying library callable, if any. */
   LibraryCallable asLibraryCallable() { this = TLibraryCallable(result) }
 
   Location getLocation() { none() }
 }
 
-/** A callable that is not synthesised. Either a CallableValue, a lambda or a module (only used to provide scopes for module variables). */
-abstract class NonLibraryDataFlowCallable extends DataFlowCallable {
-  /** Gets a callable value for this callable, if one exists. */
-  abstract CallableValue getCallableValue();
-
-  abstract CallNode getANonLibraryCall();
-
-  final override CallNode getACall() { result = this.getANonLibraryCall() }
-}
-
 /** A class representing a callable value. */
-class DataFlowCallableValue extends NonLibraryDataFlowCallable, TCallableValue {
+class DataFlowCallableValue extends DataFlowCallable, TCallableValue {
   CallableValue callable;
 
   DataFlowCallableValue() { this = TCallableValue(callable) }
 
   override string toString() { result = callable.toString() }
 
-  override CallNode getANonLibraryCall() { result = callable.getACall() }
+  override CallNode getACall() { result = callable.getACall() }
 
   override Scope getScope() { result = callable.getScope() }
 
@@ -358,14 +350,14 @@ class DataFlowCallableValue extends NonLibraryDataFlowCallable, TCallableValue {
 }
 
 /** A class representing a callable lambda. */
-class DataFlowLambda extends NonLibraryDataFlowCallable, TLambda {
+class DataFlowLambda extends DataFlowCallable, TLambda {
   Function lambda;
 
   DataFlowLambda() { this = TLambda(lambda) }
 
   override string toString() { result = lambda.toString() }
 
-  override CallNode getANonLibraryCall() { result = this.getCallableValue().getACall() }
+  override CallNode getACall() { result = this.getCallableValue().getACall() }
 
   override Scope getScope() { result = lambda.getEvaluatingScope() }
 
@@ -381,14 +373,14 @@ class DataFlowLambda extends NonLibraryDataFlowCallable, TLambda {
 }
 
 /** A class representing the scope in which a `ModuleVariableNode` appears. */
-class DataFlowModuleScope extends NonLibraryDataFlowCallable, TModule {
+class DataFlowModuleScope extends DataFlowCallable, TModule {
   Module mod;
 
   DataFlowModuleScope() { this = TModule(mod) }
 
   override string toString() { result = mod.toString() }
 
-  override CallNode getANonLibraryCall() { none() }
+  override CallNode getACall() { none() }
 
   override Scope getScope() { result = mod }
 
@@ -525,7 +517,7 @@ class NormalCall extends DataFlowSourceCall, TNormalCall {
  * `self` parameter, and special method calls have special argument passing.
  */
 class FunctionCall extends NormalCall {
-  NonLibraryDataFlowCallable callable;
+  DataFlowCallableValue callable;
 
   FunctionCall() {
     call = any(FunctionValue f).getAFunctionCall() and
@@ -539,7 +531,7 @@ class FunctionCall extends NormalCall {
 
 /** A call to a lambda. */
 class LambdaCall extends NormalCall {
-  NonLibraryDataFlowCallable callable;
+  DataFlowLambda callable;
 
   LambdaCall() {
     call = callable.getACall() and
@@ -695,6 +687,19 @@ class SummaryParameterNode extends ParameterNodeImpl, TSummaryParameterNode {
   }
 
   override DataFlowCallable getEnclosingCallable() { result.asLibraryCallable() = sc }
+
+  override string toString() { result = "parameter " + pos + " of " + sc }
+
+  // Hack to return "empty location"
+  override predicate hasLocationInfo(
+    string file, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    file = "" and
+    startline = 0 and
+    startcolumn = 0 and
+    endline = 0 and
+    endcolumn = 0
+  }
 }
 
 /** A data-flow node used to model flow summaries. */
@@ -707,6 +712,17 @@ class SummaryNode extends Node, TSummaryNode {
   override DataFlowCallable getEnclosingCallable() { result.asLibraryCallable() = c }
 
   override string toString() { result = "[summary] " + state + " in " + c }
+
+  // Hack to return "empty location"
+  override predicate hasLocationInfo(
+    string file, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    file = "" and
+    startline = 0 and
+    startcolumn = 0 and
+    endline = 0 and
+    endcolumn = 0
+  }
 }
 
 private class SummaryReturnNode extends SummaryNode, ReturnNode {

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

@@ -139,6 +139,8 @@ module SyntheticPostUpdateNode {
   Node argumentPreUpdateNode() {
     result = any(FunctionCall c).getArg(_)
     or
+    result = any(LambdaCall c).getArg(_)
+    or
     // Avoid argument 0 of method calls as those have read post-update nodes.
     exists(MethodCall c, int n | n > 0 | result = c.getArg(n))
     or
@@ -153,14 +155,11 @@ module SyntheticPostUpdateNode {
     )
   }
 
-  predicate resolvedCall(CallNode call) {
-    call = any(FunctionValue f).getAFunctionCall()
-    or
-    call = any(FunctionValue f).getAMethodCall()
-    or
-    call = any(ClassValue c | not c.isAbsent()).getACall()
+  /** Holds if `call` can be resolved as anormal call */
+  private predicate resolvedCall(CallNode call) {
+    call = any(DataFlowCallableValue cv).getACall()
     or
-    call instanceof SpecialMethodCallNode
+    call = any(DataFlowLambda l).getACall()
   }
 
   /** Gets the pre-update node associated with a store. This is used for when an object might have its value changed after a store. */

python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll

@@ -50,7 +50,7 @@ deprecated class SafeExternalAPI = SafeExternalApi;
 
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalApi extends SafeExternalApi {
-  override DataFlowPrivate::NonLibraryDataFlowCallable getSafeCallable() {
+  override DataFlowPrivate::DataFlowCallable getSafeCallable() {
     exists(CallableValue cv | cv = result.getCallableValue() |
       cv = Value::named(["len", "isinstance", "getattr", "hasattr"])
       or
@@ -65,7 +65,7 @@ private class DefaultSafeExternalApi extends SafeExternalApi {
 
 /** A node representing data being passed to an external API through a call. */
 class ExternalApiDataNode extends DataFlow::Node {
-  DataFlowPrivate::NonLibraryDataFlowCallable callable;
+  DataFlowPrivate::DataFlowCallable callable;
   int i;
 
   ExternalApiDataNode() {
@@ -156,7 +156,7 @@ class ExternalApiUsedWithUntrustedData extends TExternalApi {
   /** Gets a textual representation of this element. */
   string toString() {
     exists(
-      DataFlowPrivate::NonLibraryDataFlowCallable callable, int index, string callableString,
+      DataFlowPrivate::DataFlowCallable callable, int index, string callableString,
       string indexString
     |
       this = TExternalApiParameter(callable, index) and

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

@@ -52,7 +52,7 @@ abstract class RoutingTest extends InlineExpectationsTest {
     result =
       toNode
           .getEnclosingCallable()
-          .(DataFlowPrivate::NonLibraryDataFlowCallable)
+          .(DataFlowPrivate::DataFlowCallable)
           .getCallableValue()
           .getScope()
           .getQualifiedName() // TODO: More robust pretty printing?

python/ql/test/experimental/dataflow/coverage/classesCallGraph.expected

@@ -1,8 +1,11 @@
+| classes.py:14:17:14:60 | ControlFlowNode for Attribute() | classes.py:14:17:14:60 | ControlFlowNode for Attribute() |
+| classes.py:14:33:14:59 | ControlFlowNode for Attribute() | classes.py:14:33:14:59 | ControlFlowNode for Attribute() |
 | classes.py:45:16:45:35 | ControlFlowNode for Attribute() | classes.py:45:16:45:35 | ControlFlowNode for Attribute() |
 | classes.py:60:17:60:27 | [pre objCreate] ControlFlowNode for With_init() | classes.py:54:18:54:21 | ControlFlowNode for self |
 | classes.py:242:9:242:24 | ControlFlowNode for set() | classes.py:242:9:242:24 | ControlFlowNode for set() |
 | classes.py:247:9:247:30 | ControlFlowNode for frozenset() | classes.py:247:9:247:30 | ControlFlowNode for frozenset() |
 | classes.py:252:9:252:28 | ControlFlowNode for dict() | classes.py:252:9:252:28 | ControlFlowNode for dict() |
+| classes.py:412:29:412:52 | ControlFlowNode for dict() | classes.py:412:29:412:52 | ControlFlowNode for dict() |
 | classes.py:559:16:559:17 | ControlFlowNode for Str | classes.py:565:5:565:22 | ControlFlowNode for Subscript |
 | classes.py:565:5:565:16 | ControlFlowNode for with_getitem | classes.py:555:21:555:24 | ControlFlowNode for self |
 | classes.py:565:18:565:21 | ControlFlowNode for arg2 | classes.py:555:27:555:29 | ControlFlowNode for key |