Merge branch 'main' into py-shell

python/ql/src/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 0.6.4
+
+No user-facing changes.
+
+## 0.6.3
+
+No user-facing changes.
+
 ## 0.6.2
 
 No user-facing changes.

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

@@ -1,48 +1,36 @@
 /**
  * Definitions for reasoning about untrusted data used in APIs defined outside the
- * database.
+ * user-written code.
  */
 
-import python
+private import python
 import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.TaintTracking
-import semmle.python.Concepts
-import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.dataflow.new.TaintTracking
+private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.ApiGraphs
 private import semmle.python.dataflow.new.internal.DataFlowPrivate as DataFlowPrivate
 private import semmle.python.dataflow.new.internal.TaintTrackingPrivate as TaintTrackingPrivate
-private import semmle.python.types.Builtins
-private import semmle.python.objects.ObjectInternal
 
-// IMPLEMENTATION NOTES:
-//
-// This query uses *both* the new data-flow library, and points-to. Why? To get this
-// finished quickly, so it can provide value for our field team and ourselves.
-//
-// In the long run, it should not need to use points-to for anything. Possibly this can
-// even be helpful in figuring out what we need from TypeTrackers and the new data-flow
-// library to be fully operational.
-//
-// At least it will allow us to provide a baseline comparison against a solution that
-// doesn't use points-to at all
-//
-// There is a few dirty things we do here:
-// 1. DataFlowPrivate: since `DataFlowCall` and `DataFlowCallable` are not exposed
-//    publicly, but we really want access to them.
-// 2. points-to: we kinda need to do this since this is what powers `DataFlowCall` and
-//    `DataFlowCallable`
-// 3. ObjectInternal: to provide better names for built-in functions and methods. If we
-//    really wanted to polish our points-to implementation, we could move this
-//    functionality into `BuiltinFunctionValue` and `BuiltinMethodValue`, but will
-//    probably require some more work: for this query, it's totally ok to use
-//    `builtins.open` for the code `open(f)`, but well, it requires a bit of thinking to
-//    figure out if that is desirable in general. I simply skipped a corner here!
-// 4. TaintTrackingPrivate: Nothing else gives us access to `defaultAdditionalTaintStep` :(
 /**
- * A callable that is considered a "safe" external API from a security perspective.
+ * An external API that is considered "safe" from a security perspective.
  */
 class SafeExternalApi extends Unit {
-  /** Gets a callable that is considered a "safe" external API from a security perspective. */
-  abstract DataFlowPrivate::DataFlowCallable getSafeCallable();
+  /**
+   * Gets a call that is considered "safe" from a security perspective. You can use API
+   * graphs to find calls to functions you know are safe.
+   *
+   * Which works even when the external library isn't extracted.
+   */
+  abstract DataFlow::CallCfgNode getSafeCall();
+
+  /**
+   * Gets a callable that is considered a "safe" external API from a security
+   * perspective.
+   *
+   * You probably want to define this as `none()` and use `getSafeCall` instead, since
+   * that can handle the external library not being extracted.
+   */
+  DataFlowPrivate::DataFlowCallable getSafeCallable() { none() }
 }
 
 /** DEPRECATED: Alias for SafeExternalApi */
@@ -50,42 +38,127 @@ deprecated class SafeExternalAPI = SafeExternalApi;
 
 /** The default set of "safe" external APIs. */
 private class DefaultSafeExternalApi extends SafeExternalApi {
-  override DataFlowPrivate::DataFlowCallable getSafeCallable() {
-    exists(CallableValue cv | cv = result.getCallableValue() |
-      cv = Value::named(["len", "isinstance", "getattr", "hasattr"])
-      or
-      exists(ClassValue cls, string attr |
-        cls = Value::named("dict") and attr in ["__getitem__", "__setitem__"]
-      |
-        cls.lookup(attr) = cv
-      )
+  override DataFlow::CallCfgNode getSafeCall() {
+    result =
+      API::builtin([
+          "len", "enumerate", "isinstance", "getattr", "hasattr", "bool", "float", "int", "repr",
+          "str", "type"
+        ]).getACall()
+  }
+}
+
+/**
+ * Gets a human readable representation of `node`.
+ *
+ * Note that this is only defined for API nodes that are allowed as external APIs,
+ * so `None.json.dumps` will for example not be allowed.
+ */
+string apiNodeToStringRepr(API::Node node) {
+  node = API::builtin(result)
+  or
+  node = API::moduleImport(result)
+  or
+  exists(API::Node base, string basename |
+    base.getDepth() < node.getDepth() and
+    basename = apiNodeToStringRepr(base) and
+    not base = API::builtin(["None", "True", "False"])
+  |
+    exists(string m | node = base.getMember(m) | result = basename + "." + m)
+    or
+    node = base.getReturn() and
+    result = basename + "()" and
+    not base.getACall() = any(SafeExternalApi safe).getSafeCall()
+    or
+    node = base.getAwaited() and
+    result = basename
+  )
+}
+
+predicate resolvedCall(CallNode call) {
+  DataFlowPrivate::resolveCall(call, _, _) or
+  DataFlowPrivate::resolveClassCall(call, _)
+}
+
+newtype TInterestingExternalApiCall =
+  TUnresolvedCall(DataFlow::CallCfgNode call) {
+    exists(call.getLocation().getFile().getRelativePath()) and
+    not resolvedCall(call.getNode()) and
+    not call = any(SafeExternalApi safe).getSafeCall()
+  } or
+  TResolvedCall(DataFlowPrivate::DataFlowCall call) {
+    exists(call.getLocation().getFile().getRelativePath()) and
+    exists(call.getCallable()) and
+    not call.getCallable() = any(SafeExternalApi safe).getSafeCallable() and
+    // ignore calls inside codebase, and ignore calls that are marked as  safe. This is
+    // only needed as long as we extract dependencies. When we stop doing that, all
+    // targets of resolved calls will be from user-written code.
+    not exists(call.getCallable().getLocation().getFile().getRelativePath()) and
+    not exists(DataFlow::CallCfgNode callCfgNode | callCfgNode.getNode() = call.getNode() |
+      any(SafeExternalApi safe).getSafeCall() = callCfgNode
+    )
+  }
+
+abstract class InterestingExternalApiCall extends TInterestingExternalApiCall {
+  /** Gets the argument at position `apos`, if any */
+  abstract DataFlow::Node getArgument(DataFlowPrivate::ArgumentPosition apos);
+
+  /** Gets a textual representation of this element. */
+  abstract string toString();
+
+  /**
+   * Gets a human-readable name for the external API.
+   */
+  abstract string getApiName();
+}
+
+class UnresolvedCall extends InterestingExternalApiCall, TUnresolvedCall {
+  DataFlow::CallCfgNode call;
+
+  UnresolvedCall() { this = TUnresolvedCall(call) }
+
+  override DataFlow::Node getArgument(DataFlowPrivate::ArgumentPosition apos) {
+    exists(int i | apos.isPositional(i) | result = call.getArg(i))
+    or
+    exists(string name | apos.isKeyword(name) | result = call.getArgByName(name))
+  }
+
+  override string toString() {
+    result = "ExternalAPI:UnresolvedCall: " + call.getNode().getNode().toString()
+  }
+
+  override string getApiName() {
+    exists(API::Node apiNode |
+      result = apiNodeToStringRepr(apiNode) and
+      apiNode.getACall() = call
+    )
+  }
+}
+
+class ResolvedCall extends InterestingExternalApiCall, TResolvedCall {
+  DataFlowPrivate::DataFlowCall dfCall;
+
+  ResolvedCall() { this = TResolvedCall(dfCall) }
+
+  override DataFlow::Node getArgument(DataFlowPrivate::ArgumentPosition apos) {
+    result = dfCall.getArgument(apos)
+  }
+
+  override string toString() {
+    result = "ExternalAPI:ResolvedCall: " + dfCall.getNode().getNode().toString()
+  }
+
+  override string getApiName() {
+    exists(DataFlow::CallCfgNode call, API::Node apiNode | dfCall.getNode() = call.getNode() |
+      result = apiNodeToStringRepr(apiNode) and
+      apiNode.getACall() = call
     )
   }
 }
 
 /** A node representing data being passed to an external API through a call. */
 class ExternalApiDataNode extends DataFlow::Node {
-  DataFlowPrivate::DataFlowCallable callable;
-  int i;
-
   ExternalApiDataNode() {
-    exists(DataFlowPrivate::DataFlowCall call |
-      exists(call.getLocation().getFile().getRelativePath())
-    |
-      callable = call.getCallable() and
-      // TODO: this ignores some complexity of keyword arguments (especially keyword-only args)
-      this = call.getArg(i)
-    ) and
-    not any(SafeExternalApi safe).getSafeCallable() = callable and
-    exists(Value cv | cv = callable.getCallableValue() |
-      cv.isAbsent()
-      or
-      cv.isBuiltin()
-      or
-      cv.(CallableValue).getScope().getLocation().getFile().inStdlib()
-      or
-      not exists(cv.(CallableValue).getScope().getLocation().getFile().getRelativePath())
-    ) and
+    exists(InterestingExternalApiCall call | this = call.getArgument(_)) and
     // Not already modeled as a taint step
     not TaintTrackingPrivate::defaultAdditionalTaintStep(this, _) and
     // for `list.append(x)`, we have a additional taint step from x -> [post] list.
@@ -95,12 +168,6 @@ class ExternalApiDataNode extends DataFlow::Node {
       TaintTrackingPrivate::defaultAdditionalTaintStep(_, post)
     )
   }
-
-  /** Gets the index for the parameter that will receive this untrusted data */
-  int getIndex() { result = i }
-
-  /** Gets the callable to which this argument is passed. */
-  DataFlowPrivate::DataFlowCallable getCallable() { result = callable }
 }
 
 /** DEPRECATED: Alias for ExternalApiDataNode */
@@ -133,19 +200,26 @@ deprecated class UntrustedExternalAPIDataNode = UntrustedExternalApiDataNode;
 
 /** An external API which is used with untrusted data. */
 private newtype TExternalApi =
-  /** An untrusted API method `m` where untrusted data is passed at `index`. */
-  TExternalApiParameter(DataFlowPrivate::DataFlowCallable callable, int index) {
-    exists(UntrustedExternalApiDataNode n |
-      callable = n.getCallable() and
-      index = n.getIndex()
+  MkExternalApi(string repr, DataFlowPrivate::ArgumentPosition apos) {
+    exists(UntrustedExternalApiDataNode ex, InterestingExternalApiCall call |
+      ex = call.getArgument(apos) and
+      repr = call.getApiName()
     )
   }
 
-/** An external API which is used with untrusted data. */
-class ExternalApiUsedWithUntrustedData extends TExternalApi {
+/** A argument of an external API which is used with untrusted data. */
+class ExternalApiUsedWithUntrustedData extends MkExternalApi {
+  string repr;
+  DataFlowPrivate::ArgumentPosition apos;
+
+  ExternalApiUsedWithUntrustedData() { this = MkExternalApi(repr, apos) }
+
   /** Gets a possibly untrusted use of this external API. */
   UntrustedExternalApiDataNode getUntrustedDataNode() {
-    this = TExternalApiParameter(result.getCallable(), result.getIndex())
+    exists(InterestingExternalApiCall call |
+      result = call.getArgument(apos) and
+      call.getApiName() = repr
+    )
   }
 
   /** Gets the number of untrusted sources used with this external API. */
@@ -154,63 +228,8 @@ class ExternalApiUsedWithUntrustedData extends TExternalApi {
   }
 
   /** Gets a textual representation of this element. */
-  string toString() {
-    exists(
-      DataFlowPrivate::DataFlowCallable callable, int index, string callableString,
-      string indexString
-    |
-      this = TExternalApiParameter(callable, index) and
-      indexString = "param " + index and
-      exists(CallableValue cv | cv = callable.getCallableValue() |
-        callableString =
-          cv.getScope().getEnclosingModule().getName() + "." + cv.getScope().getQualifiedName()
-        or
-        not exists(cv.getScope()) and
-        (
-          cv instanceof BuiltinFunctionValue and
-          callableString = pretty_builtin_function_value(cv)
-          or
-          cv instanceof BuiltinMethodValue and
-          callableString = pretty_builtin_method_value(cv)
-          or
-          not cv instanceof BuiltinFunctionValue and
-          not cv instanceof BuiltinMethodValue and
-          callableString = cv.toString()
-        )
-      ) and
-      result = callableString + " [" + indexString + "]"
-    )
-  }
+  string toString() { result = repr + " [" + apos + "]" }
 }
 
 /** DEPRECATED: Alias for ExternalApiUsedWithUntrustedData */
 deprecated class ExternalAPIUsedWithUntrustedData = ExternalApiUsedWithUntrustedData;
-
-/** Gets the fully qualified name for the `BuiltinFunctionValue` bfv. */
-private string pretty_builtin_function_value(BuiltinFunctionValue bfv) {
-  exists(Builtin b | b = bfv.(BuiltinFunctionObjectInternal).getBuiltin() |
-    result = prefix_with_module_if_found(b)
-  )
-}
-
-/** Gets the fully qualified name for the `BuiltinMethodValue` bmv. */
-private string pretty_builtin_method_value(BuiltinMethodValue bmv) {
-  exists(Builtin b | b = bmv.(BuiltinMethodObjectInternal).getBuiltin() |
-    exists(Builtin cls | cls.isClass() and cls.getMember(b.getName()) = b |
-      result = prefix_with_module_if_found(cls) + "." + b.getName()
-    )
-    or
-    not exists(Builtin cls | cls.isClass() and cls.getMember(b.getName()) = b) and
-    result = b.getName()
-  )
-}
-
-/** Helper predicate that tries to adds module qualifier to `b`. Will succeed even if module not found. */
-private string prefix_with_module_if_found(Builtin b) {
-  exists(Builtin mod | mod.isModule() and mod.getMember(b.getName()) = b |
-    result = mod.getName() + "." + b.getName()
-  )
-  or
-  not exists(Builtin mod | mod.isModule() and mod.getMember(b.getName()) = b) and
-  result = b.getName()
-}

python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIsUsedWithUntrustedData.qhelp

@@ -11,11 +11,9 @@ relevant for security analysis of this application.</p>
 
 <p>An external API is defined as a call to a method that is not defined in the source
 code, and is not modeled as a taint step in the default taint library. External APIs may
-be from the Python standard library or dependencies. The query will report the fully qualified name,
-along with <code>[param x]</code>, where <code>x</code> indicates the position of
-the parameter receiving the untrusted data. Note that for methods and
-<code>classmethod</code>s, parameter 0 represents the class instance or class itself
-respectively.</p>
+be from the Python standard library or dependencies. The query will report the fully
+qualified name, along with <code>[position index]</code> or <code>[keyword name]</code>,
+to indicate the argument passing the untrusted data.</p>
 
 <p>Note that an excepted sink might not be included in the results, if it also defines a
 taint step. This is the case for <code>pickle.loads</code> which is a sink for the
@@ -24,8 +22,6 @@ Unsafe Deserialization query, but is also a taint step for other queries.</p>
 <p>Note: Compared to the Java version of this query, we currently do not give special
 care to methods that are overridden in the source code.</p>
 
-<p>Note: Currently this query will only report results for external packages that are extracted.</p>
-
 </overview>
 <recommendation>
 

python/ql/src/Security/CWE-020-ExternalAPIs/UntrustedDataToExternalAPI.qhelp

@@ -11,11 +11,9 @@ be modeled as either taint steps, or sinks for specific problems.</p>
 
 <p>An external API is defined as a call to a method that is not defined in the source
 code, and is not modeled as a taint step in the default taint library. External APIs may
-be from the Python standard library or dependencies. The query will report the fully qualified name,
-along with <code>[param x]</code>, where <code>x</code> indicates the position of
-the parameter receiving the untrusted data. Note that for methods and
-<code>classmethod</code>s, parameter 0 represents the class instance or class itself
-respectively.</p>
+be from the Python standard library or dependencies. The query will report the fully
+qualified name, along with <code>[position index]</code> or <code>[keyword name]</code>,
+to indicate the argument passing the untrusted data.</p>
 
 <p>Note that an excepted sink might not be included in the results, if it also defines a
 taint step. This is the case for <code>pickle.loads</code> which is a sink for the
@@ -24,8 +22,6 @@ Unsafe Deserialization query, but is also a taint step for other queries.</p>
 <p>Note: Compared to the Java version of this query, we currently do not give special
 care to methods that are overridden in the source code.</p>
 
-<p>Note: Currently this query will only report results for external packages that are extracted.</p>
-
 </overview>
 <recommendation>
 

python/ql/src/Security/CWE-022/examples/tainted_path.py

@@ -1,36 +1,30 @@
 import os.path
+from flask import Flask, request, abort
 
+app = Flask(__name__)
 
-urlpatterns = [
-    # Route to user_picture
-    url(r'^user-pic1$', user_picture1, name='user-picture1'),
-    url(r'^user-pic2$', user_picture2, name='user-picture2'),
-    url(r'^user-pic3$', user_picture3, name='user-picture3')
-]
-
-
-def user_picture1(request):
-    """A view that is vulnerable to malicious file access."""
-    filename = request.GET.get('p')
+@app.route("/user_picture1")
+def user_picture1():
+    filename = request.args.get('p')
     # BAD: This could read any file on the file system
     data = open(filename, 'rb').read()
-    return HttpResponse(data)
+    return data
 
-def user_picture2(request):
-    """A view that is vulnerable to malicious file access."""
+@app.route("/user_picture2")
+def user_picture2():
     base_path = '/server/static/images'
-    filename = request.GET.get('p')
+    filename = request.args.get('p')
     # BAD: This could still read any file on the file system
     data = open(os.path.join(base_path, filename), 'rb').read()
-    return HttpResponse(data)
+    return data
 
-def user_picture3(request):
-    """A view that is not vulnerable to malicious file access."""
+@app.route("/user_picture3")
+def user_picture3():
     base_path = '/server/static/images'
-    filename = request.GET.get('p')
+    filename = request.args.get('p')
     #GOOD -- Verify with normalised version of path
     fullpath = os.path.normpath(os.path.join(base_path, filename))
     if not fullpath.startswith(base_path):
-        raise SecurityException()
+        raise Exception("not allowed")
     data = open(fullpath, 'rb').read()
-    return HttpResponse(data)
+    return data

python/ql/src/Security/CWE-022/examples/tarslip_bad.py

@@ -1,7 +1,7 @@
-
+import sys
 import tarfile
 
-with tarfile.open('archive.zip') as tar:
+with tarfile.open(sys.argv[1]) as tar:
     #BAD : This could write any file on the filesystem.
     for entry in tar:
         tar.extract(entry, "/tmp/unpack/")

python/ql/src/Security/CWE-022/examples/tarslip_good.py

@@ -1,8 +1,8 @@
-
+import sys
 import tarfile
 import os.path
 
-with tarfile.open('archive.zip') as tar:
+with tarfile.open(sys.argv[1]) as tar:
     for entry in tar:
         #GOOD: Check that entry is safe
         if os.path.isabs(entry.name) or ".." in entry.name:

python/ql/src/Security/CWE-327/FluentApiModel.qll

@@ -20,48 +20,100 @@ import TlsLibraryModel
  * Since we really want "the last unrestriction, not nullified by a restriction",
  * we also disallow flow into restrictions.
  */
-class InsecureContextConfiguration extends DataFlow::Configuration {
-  TlsLibrary library;
-  ProtocolVersion tracked_version;
+module InsecureContextConfiguration2 implements DataFlow::StateConfigSig {
+  private newtype TFlowState =
+    TMkFlowState(TlsLibrary library, int bits) {
+      bits in [0 .. max(any(ProtocolVersion v).getBit()) * 2 - 1]
+    }
 
-  InsecureContextConfiguration() {
-    this = library + "Allows" + tracked_version and
-    tracked_version.isInsecure()
+  class FlowState extends TFlowState {
+    int getBits() { this = TMkFlowState(_, result) }
+
+    TlsLibrary getLibrary() { this = TMkFlowState(result, _) }
+
+    predicate allowsInsecureVersion(ProtocolVersion v) {
+      v.isInsecure() and this.getBits().bitAnd(v.getBit()) != 0
+    }
+
+    string toString() {
+      result =
+        "FlowState(" + this.getLibrary().toString() + ", " +
+          concat(ProtocolVersion v | this.allowsInsecureVersion(v) | v, ", ") + ")"
+    }
   }
 
-  ProtocolVersion getTrackedVersion() { result = tracked_version }
-
-  override predicate isSource(DataFlow::Node source) { this.isUnrestriction(source) }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink = library.connection_creation().getContext()
-  }
-
-  override predicate isBarrierIn(DataFlow::Node node) {
-    this.isRestriction(node)
+  private predicate relevantState(FlowState state) {
+    isSource(_, state)
     or
-    this.isUnrestriction(node)
-  }
-
-  private predicate isRestriction(DataFlow::Node node) {
-    exists(ProtocolRestriction r |
-      r = library.protocol_restriction() and
-      r.getRestriction() = tracked_version
-    |
-      node = r.getContext()
+    exists(FlowState state0 | relevantState(state0) |
+      exists(ProtocolRestriction r |
+        r = state0.getLibrary().protocol_restriction() and
+        state.getBits() = state0.getBits().bitAnd(sum(r.getRestriction().getBit()).bitNot()) and
+        state0.getLibrary() = state.getLibrary()
+      )
+      or
+      exists(ProtocolUnrestriction pu |
+        pu = state0.getLibrary().protocol_unrestriction() and
+        state.getBits() = state0.getBits().bitOr(sum(pu.getUnrestriction().getBit())) and
+        state0.getLibrary() = state.getLibrary()
+      )
     )
   }
 
-  private predicate isUnrestriction(DataFlow::Node node) {
-    exists(ProtocolUnrestriction pu |
-      pu = library.protocol_unrestriction() and
-      pu.getUnrestriction() = tracked_version
-    |
-      node = pu.getContext()
+  predicate isSource(DataFlow::Node source, FlowState state) {
+    exists(ProtocolFamily family |
+      source = state.getLibrary().unspecific_context_creation(family) and
+      state.getBits() = family.getBits()
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink, FlowState state) {
+    sink = state.getLibrary().connection_creation().getContext() and
+    state.allowsInsecureVersion(_)
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+  ) {
+    DataFlow::localFlowStep(node1, node2) and
+    relevantState(state1) and
+    (
+      exists(ProtocolRestriction r |
+        r = state1.getLibrary().protocol_restriction() and
+        node2 = r.getContext() and
+        state2.getBits() = state1.getBits().bitAnd(sum(r.getRestriction().getBit()).bitNot()) and
+        state1.getLibrary() = state2.getLibrary()
+      )
+      or
+      exists(ProtocolUnrestriction pu |
+        pu = state1.getLibrary().protocol_unrestriction() and
+        node2 = pu.getContext() and
+        state2.getBits() = state1.getBits().bitOr(sum(pu.getUnrestriction().getBit())) and
+        state1.getLibrary() = state2.getLibrary()
+      )
+    )
+  }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) {
+    relevantState(state) and
+    (
+      exists(ProtocolRestriction r |
+        r = state.getLibrary().protocol_restriction() and
+        node = r.getContext() and
+        state.allowsInsecureVersion(r.getRestriction())
+      )
+      or
+      exists(ProtocolUnrestriction pu |
+        pu = state.getLibrary().protocol_unrestriction() and
+        node = pu.getContext() and
+        not state.allowsInsecureVersion(pu.getUnrestriction())
+      )
     )
   }
 }
 
+private module InsecureContextFlow = DataFlow::MakeWithState<InsecureContextConfiguration2>;
+
 /**
  * Holds if `conectionCreation` marks the creation of a connection based on the contex
  * found at `contextOrigin` and allowing `insecure_version`.
@@ -74,8 +126,11 @@ predicate unsafe_connection_creation_with_context(
   boolean specific
 ) {
   // Connection created from a context allowing `insecure_version`.
-  exists(InsecureContextConfiguration c | c.hasFlow(contextOrigin, connectionCreation) |
-    insecure_version = c.getTrackedVersion() and
+  exists(InsecureContextFlow::PathNode src, InsecureContextFlow::PathNode sink |
+    InsecureContextFlow::hasFlowPath(src, sink) and
+    src.getNode() = contextOrigin and
+    sink.getNode() = connectionCreation and
+    sink.getState().allowsInsecureVersion(insecure_version) and
     specific = false
   )
   or

python/ql/src/Security/CWE-327/PyOpenSSL.qll

@@ -51,8 +51,9 @@ class SetOptionsCall extends ProtocolRestriction, DataFlow::CallCfgNode {
   }
 }
 
-class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation {
-  UnspecificPyOpenSslContextCreation() { library instanceof PyOpenSsl }
+class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation
+{
+  // UnspecificPyOpenSslContextCreation() { library instanceof PyOpenSsl }
 }
 
 class PyOpenSsl extends TlsLibrary {

python/ql/src/Security/CWE-327/Ssl.qll

@@ -162,23 +162,21 @@ class ContextSetVersion extends ProtocolRestriction, ProtocolUnrestriction, Data
 }
 
 class UnspecificSslContextCreation extends SslContextCreation, UnspecificContextCreation {
-  UnspecificSslContextCreation() { library instanceof Ssl }
-
-  override ProtocolVersion getUnrestriction() {
-    result = UnspecificContextCreation.super.getUnrestriction() and
-    // These are turned off by default since Python 3.6
-    // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext
-    not result in ["SSLv2", "SSLv3"]
-  }
+  // UnspecificSslContextCreation() { library instanceof Ssl }
+  // override ProtocolVersion getUnrestriction() {
+  //   result = UnspecificContextCreation.super.getUnrestriction() and
+  //   // These are turned off by default since Python 3.6
+  //   // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext
+  //   not result in ["SSLv2", "SSLv3"]
+  // }
 }
 
-class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation, ProtocolUnrestriction {
-  override DataFlow::Node getContext() { result = this }
-
-  // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context
-  override ProtocolVersion getUnrestriction() {
-    result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
-  }
+class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation {
+  // override DataFlow::Node getContext() { result = this }
+  // // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context
+  // override ProtocolVersion getUnrestriction() {
+  //   result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
+  // }
 }
 
 class Ssl extends TlsLibrary {

python/ql/src/Security/CWE-327/TlsLibraryModel.qll

@@ -15,18 +15,45 @@ class ProtocolVersion extends string {
     or
     this = "TLSv1" and version = ["TLSv1_1", "TLSv1_2", "TLSv1_3"]
     or
-    this = ["TLSv1", "TLSv1_1"] and version = ["TLSv1_2", "TLSv1_3"]
+    this = "TLSv1_1" and version = ["TLSv1_2", "TLSv1_3"]
     or
-    this = ["TLSv1", "TLSv1_1", "TLSv1_2"] and version = "TLSv1_3"
+    this = "TLSv1_2" and version = "TLSv1_3"
   }
 
   /** Holds if this protocol version is known to be insecure. */
   predicate isInsecure() { this in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1"] }
+
+  /** Gets the bit mask for this protocol version. */
+  int getBit() {
+    this = "SSLv2" and result = 1
+    or
+    this = "SSLv3" and result = 2
+    or
+    this = "TLSv1" and result = 4
+    or
+    this = "TLSv1_1" and result = 8
+    or
+    this = "TLSv1_2" and result = 16
+    or
+    this = "TLSv1_3" and result = 32
+  }
+
+  /** Gets the protocol family for this protocol version. */
+  ProtocolFamily getFamily() {
+    result = "SSLv23" and this in ["SSLv2", "SSLv3"]
+    or
+    result = "TLS" and this in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
+  }
 }
 
 /** An unspecific protocol version */
 class ProtocolFamily extends string {
   ProtocolFamily() { this in ["SSLv23", "TLS"] }
+
+  /** Gets the bit mask for this protocol family. */
+  int getBits() {
+    result = sum(ProtocolVersion version | version.getFamily() = this | version.getBit())
+  }
 }
 
 /** The creation of a context. */
@@ -63,21 +90,14 @@ abstract class ProtocolUnrestriction extends DataFlow::Node {
  * A context is being created with a range of allowed protocols.
  * This also serves as unrestricting these protocols.
  */
-abstract class UnspecificContextCreation extends ContextCreation, ProtocolUnrestriction {
-  TlsLibrary library;
-  ProtocolFamily family;
-
-  UnspecificContextCreation() { this.getProtocol() = family }
-
-  override DataFlow::CfgNode getContext() { result = this }
-
-  override ProtocolVersion getUnrestriction() {
-    // There is only one family, the two names are aliases in OpenSSL.
-    // see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955
-    family in ["SSLv23", "TLS"] and
-    // see https://docs.python.org/3/library/ssl.html#ssl-contexts
-    result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
-  }
+abstract class UnspecificContextCreation extends ContextCreation {
+  // override ProtocolVersion getUnrestriction() {
+  //   // There is only one family, the two names are aliases in OpenSSL.
+  //   // see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955
+  //   family in ["SSLv23", "TLS"] and
+  //   // see https://docs.python.org/3/library/ssl.html#ssl-contexts
+  //   result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
+  // }
 }
 
 /** A model of a SSL/TLS library. */

python/ql/src/change-notes/released/0.6.3.md

@@ -0,0 +1,3 @@
+## 0.6.3
+
+No user-facing changes.

python/ql/src/change-notes/released/0.6.4.md

@@ -0,0 +1,3 @@
+## 0.6.4
+
+No user-facing changes.

python/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.6.2
+lastReleaseVersion: 0.6.4

python/ql/src/experimental/Security/CWE-022bis/UnsafeUnpack.qhelp

@@ -0,0 +1,56 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Extracting files from a malicious tarball without validating that the destination file path
+is within the destination directory using <code>shutil.unpack_archive()</code> can cause files outside the 
+destination directory to be overwritten, due to the possible presence of directory traversal elements
+ (<code>..</code>) in archive path names.</p>
+
+<p>Tarball contain archive entries representing each file in the archive. These entries
+include a file path for the entry, but these file paths are not restricted and may contain
+unexpected special elements such as the directory traversal element (<code>..</code>). If these
+file paths are used to determine an output file to write the contents of the archive item to, then
+the file may be written to an unexpected location. This can result in sensitive information being
+revealed or deleted, or an attacker being able to influence behavior by modifying unexpected
+files.</p>
+
+<p>For example, if a tarball contains a file entry <code>../sneaky-file.txt</code>, and the tarball
+is extracted to the directory <code>/tmp/tmp123</code>, then naively combining the paths would result
+in an output file path of <code>/tmp/tmp123/../sneaky-file.txt</code>, which would cause the file to be
+written to <code>/tmp/</code>.</p>
+
+</overview>
+<recommendation>
+
+<p>Ensure that output paths constructed from tarball entries are validated
+to prevent writing files to unexpected locations.</p>
+
+<p>Consider using a safer module, such as: <code>zipfile</code></p>
+
+</recommendation>
+
+<example>
+<p>
+In this example an archive is extracted without validating file paths.
+</p>
+
+<sample src="examples/HIT_UnsafeUnpack.py" />
+
+<p>To fix this vulnerability, we need to call the function <code>tarfile.extract()</code> 
+  on each <code>member</code> after verifying that it does not contain either <code>..</code> or startswith <code>/</code>.
+</p>
+
+<sample src="examples/NoHIT_UnsafeUnpack.py" />
+
+</example>
+<references>
+
+<li>
+  Shutil official documentation
+  <a href="https://docs.python.org/3/library/shutil.html?highlight=unpack_archive#shutil.unpack_archive">shutil.unpack_archive() warning.</a>
+  </li>
+</references>
+</qhelp>

python/ql/src/experimental/Security/CWE-022bis/UnsafeUnpack.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Arbitrary file write during a tarball extraction from a user controlled source
+ * @description Extracting files from a potentially malicious tarball using `shutil.unpack_archive()` without validating
+ *              that the destination file path is within the destination directory can cause files outside
+ *              the destination directory to be overwritten. More precisely, if the tarball comes from a user controlled
+ *              location either a remote one or cli argument.
+ * @kind path-problem
+ * @id py/unsafe-unpacking
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision medium
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-022
+ */
+
+import python
+import experimental.Security.UnsafeUnpackQuery
+import DataFlow::PathGraph
+
+from UnsafeUnpackingConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Unsafe extraction from a malicious tarball retrieved from a remote location."

python/ql/src/experimental/Security/CWE-022bis/examples/HIT_UnsafeUnpack.py

@@ -0,0 +1,12 @@
+import requests
+import shutil
+
+url = "https://www.someremote.location/tarball.tar.gz"
+response = requests.get(url, stream=True)
+
+tarpath = "/tmp/tmp456/tarball.tar.gz"
+with open(tarpath, "wb") as f:
+      f.write(response.raw.read())
+
+untarredpath = "/tmp/tmp123"
+shutil.unpack_archive(tarpath, untarredpath)

python/ql/src/experimental/Security/CWE-022bis/examples/NoHIT_UnsafeUnpack.py

@@ -0,0 +1,17 @@
+import requests
+import tarfile
+
+url = "https://www.someremote.location/tarball.tar.gz"
+response = requests.get(url, stream=True)
+
+tarpath = "/tmp/tmp456/tarball.tar.gz"
+with open(tarpath, "wb") as f:
+      f.write(response.raw.read())
+
+untarredpath = "/tmp/tmp123"
+with tarfile.open(tarpath) as tar:
+	for member in tar.getmembers():
+		if member.name.startswith("/") or ".." in member.name:
+			raise Exception("Path traversal identified in tarball")
+
+		tar.extract(untarredpath, member)

python/ql/src/experimental/Security/CWE-348/ClientSuppliedIpUsedInSecurityCheckLib.qll

@@ -16,7 +16,8 @@ private import semmle.python.frameworks.Tornado
 abstract class ClientSuppliedIpUsedInSecurityCheck extends DataFlow::Node { }
 
 private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
-  DataFlow::MethodCallNode {
+  DataFlow::MethodCallNode
+{
   FlaskClientSuppliedIpUsedInSecurityCheck() {
     this = Flask::request().getMember("headers").getMember(["get", "get_all", "getlist"]).getACall() and
     this.getArg(0).asExpr().(StrConst).getText().toLowerCase() = clientIpParameterName()
@@ -24,7 +25,8 @@ private class FlaskClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpU
 }
 
 private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
-  DataFlow::MethodCallNode {
+  DataFlow::MethodCallNode
+{
   DjangoClientSuppliedIpUsedInSecurityCheck() {
     exists(DataFlow::Node req, DataFlow::AttrRead headers |
       // a call to request.headers.get or request.META.get
@@ -38,7 +40,8 @@ private class DjangoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIp
 }
 
 private class TornadoClientSuppliedIpUsedInSecurityCheck extends ClientSuppliedIpUsedInSecurityCheck,
-  DataFlow::MethodCallNode {
+  DataFlow::MethodCallNode
+{
   TornadoClientSuppliedIpUsedInSecurityCheck() {
     // a call to self.request.headers.get or self.request.headers.get_list inside a tornado requesthandler
     exists(

python/ql/src/experimental/Security/UnsafeUnpackQuery.qll

@@ -0,0 +1,213 @@
+/**
+ *  Provides a taint-tracking configuration for detecting "UnsafeUnpacking" vulnerabilities.
+ */
+
+import python
+import semmle.python.Concepts
+import semmle.python.dataflow.new.internal.DataFlowPublic
+import semmle.python.ApiGraphs
+import semmle.python.dataflow.new.TaintTracking
+import semmle.python.frameworks.Stdlib
+import semmle.python.dataflow.new.RemoteFlowSources
+
+/**
+ * Handle those three cases of Tarfile opens:
+ *  - `tarfile.open()`
+ *  - `tarfile.TarFile()`
+ *  - `MKtarfile.Tarfile.open()`
+ */
+API::Node tarfileOpen() {
+  result in [
+      API::moduleImport("tarfile").getMember(["open", "TarFile"]),
+      API::moduleImport("tarfile").getMember("TarFile").getASubclass().getMember("open")
+    ]
+}
+
+/**
+ * A class for handling the previous three cases, plus the use of `closing` in with the previous cases
+ */
+class AllTarfileOpens extends API::CallNode {
+  AllTarfileOpens() {
+    this = tarfileOpen().getACall()
+    or
+    exists(API::Node closing, Node arg |
+      closing = API::moduleImport("contextlib").getMember("closing") and
+      this = closing.getACall() and
+      arg = this.getArg(0) and
+      arg = tarfileOpen().getACall()
+    )
+  }
+}
+
+class UnsafeUnpackingConfig extends TaintTracking::Configuration {
+  UnsafeUnpackingConfig() { this = "UnsafeUnpackingConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    // A source coming from a remote location
+    source instanceof RemoteFlowSource
+    or
+    // A source coming from a CLI argparse module
+    // see argparse: https://docs.python.org/3/library/argparse.html
+    exists(MethodCallNode args |
+      args = source.(AttrRead).getObject().getALocalSource() and
+      args =
+        API::moduleImport("argparse")
+            .getMember("ArgumentParser")
+            .getReturn()
+            .getMember("parse_args")
+            .getACall()
+    )
+    or
+    // A source catching an S3 file download
+    // see boto3: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.download_file
+    source =
+      API::moduleImport("boto3")
+          .getMember("client")
+          .getReturn()
+          .getMember(["download_file", "download_fileobj"])
+          .getACall()
+          .getArg(2)
+    or
+    // A source catching an S3 file download
+    // see boto3: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html
+    source =
+      API::moduleImport("boto3")
+          .getMember("Session")
+          .getReturn()
+          .getMember("client")
+          .getReturn()
+          .getMember(["download_file", "download_fileobj"])
+          .getACall()
+          .getArg(2)
+    or
+    // A source download a file using wget
+    // see wget: https://pypi.org/project/wget/
+    exists(API::CallNode mcn |
+      mcn = API::moduleImport("wget").getMember("download").getACall() and
+      if exists(mcn.getArg(1)) then source = mcn.getArg(1) else source = mcn.getReturn().asSource()
+    )
+    or
+    // catch the Django uploaded files as a source
+    // see HttpRequest.FILES: https://docs.djangoproject.com/en/4.1/ref/request-response/#django.http.HttpRequest.FILES
+    source.(AttrRead).getAttributeName() = "FILES"
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    (
+      // A sink capturing method calls to `unpack_archive`.
+      sink = API::moduleImport("shutil").getMember("unpack_archive").getACall().getArg(0)
+      or
+      // A sink capturing method calls to `extractall` without `members` argument.
+      // For a call to `file.extractall` without `members` argument, `file` is considered a sink.
+      exists(MethodCallNode call, AllTarfileOpens atfo |
+        call = atfo.getReturn().getMember("extractall").getACall() and
+        not exists(call.getArgByName("members")) and
+        sink = call.getObject()
+      )
+      or
+      // A sink capturing method calls to `extractall` with `members` argument.
+      // For a call to `file.extractall` with `members` argument, `file` is considered a sink if not
+      // a the `members` argument contains a NameConstant as None, a List or call to the method `getmembers`.
+      // Otherwise, the argument of `members` is considered a sink.
+      exists(MethodCallNode call, Node arg, AllTarfileOpens atfo |
+        call = atfo.getReturn().getMember("extractall").getACall() and
+        arg = call.getArgByName("members") and
+        if
+          arg.asCfgNode() instanceof NameConstantNode or
+          arg.asCfgNode() instanceof ListNode
+        then sink = call.getObject()
+        else
+          if arg.(MethodCallNode).getMethodName() = "getmembers"
+          then sink = arg.(MethodCallNode).getObject()
+          else sink = call.getArgByName("members")
+      )
+      or
+      // An argument to `extract` is considered a sink.
+      exists(AllTarfileOpens atfo |
+        sink = atfo.getReturn().getMember("extract").getACall().getArg(0)
+      )
+      or
+      //An argument to `_extract_member` is considered a sink.
+      exists(MethodCallNode call, AllTarfileOpens atfo |
+        call = atfo.getReturn().getMember("_extract_member").getACall() and
+        call.getArg(1).(AttrRead).accesses(sink, "name")
+      )
+    ) and
+    not sink.getScope().getLocation().getFile().inStdlib()
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    // Reading the response
+    nodeTo.(MethodCallNode).calls(nodeFrom, "read")
+    or
+    // Open a file for access
+    exists(MethodCallNode cn |
+      cn.calls(nodeTo, "open") and
+      cn.flowsTo(nodeFrom)
+    )
+    or
+    // Open a file for access using builtin
+    exists(API::CallNode cn |
+      cn = API::builtin("open").getACall() and
+      nodeTo = cn.getArg(0) and
+      cn.flowsTo(nodeFrom)
+    )
+    or
+    // Write access
+    exists(MethodCallNode cn |
+      cn.calls(nodeTo, "write") and
+      nodeFrom = cn.getArg(0)
+    )
+    or
+    // Retrieve Django uploaded files
+    // see getlist(): https://docs.djangoproject.com/en/4.1/ref/request-response/#django.http.QueryDict.getlist
+    // see chunks(): https://docs.djangoproject.com/en/4.1/ref/files/uploads/#django.core.files.uploadedfile.UploadedFile.chunks
+    nodeTo.(MethodCallNode).calls(nodeFrom, ["getlist", "get", "chunks"])
+    or
+    // Considering the use of "fs"
+    // see fs: https://docs.djangoproject.com/en/4.1/ref/files/storage/#the-filesystemstorage-class
+    nodeTo =
+      API::moduleImport("django")
+          .getMember("core")
+          .getMember("files")
+          .getMember("storage")
+          .getMember("FileSystemStorage")
+          .getReturn()
+          .getMember(["save", "path"])
+          .getACall() and
+    nodeFrom = nodeTo.(MethodCallNode).getArg(0)
+    or
+    // Accessing the name or raw content
+    nodeTo.(AttrRead).accesses(nodeFrom, ["name", "raw"])
+    or
+    // Join the base_dir to the filename
+    nodeTo = API::moduleImport("os").getMember("path").getMember("join").getACall() and
+    nodeFrom = nodeTo.(API::CallNode).getArg(1)
+    or
+    // Go through an Open for a Tarfile
+    nodeTo = tarfileOpen().getACall() and nodeFrom = nodeTo.(MethodCallNode).getArg(0)
+    or
+    // Handle the case where the getmembers is used.
+    nodeTo.(MethodCallNode).calls(nodeFrom, "getmembers") and
+    nodeFrom instanceof AllTarfileOpens
+    or
+    // To handle the case of `with closing(tarfile.open()) as file:`
+    // we add a step from the first argument of `closing` to the call to `closing`,
+    // whenever that first argument is a return of `tarfile.open()`.
+    nodeTo = API::moduleImport("contextlib").getMember("closing").getACall() and
+    nodeFrom = nodeTo.(API::CallNode).getArg(0) and
+    nodeFrom = tarfileOpen().getReturn().getAValueReachableFromSource()
+    or
+    // see Path : https://docs.python.org/3/library/pathlib.html#pathlib.Path
+    nodeTo = API::moduleImport("pathlib").getMember("Path").getACall() and
+    nodeFrom = nodeTo.(API::CallNode).getArg(0)
+    or
+    // Use of absolutepath
+    // see absolute : https://docs.python.org/3/library/pathlib.html#pathlib.Path.absolute
+    exists(API::CallNode mcn |
+      mcn = API::moduleImport("pathlib").getMember("Path").getACall() and
+      nodeTo = mcn.getAMethodCall("absolute") and
+      nodeFrom = mcn.getArg(0)
+    )
+  }
+}

python/ql/src/experimental/semmle/python/security/InsecureRandomnessCustomizations.qll

@@ -59,12 +59,11 @@ module InsecureRandomness {
    */
   class RandomFnSink extends Sink {
     RandomFnSink() {
-      exists(DataFlowCallable randomFn |
-        randomFn
-            .getName()
+      exists(Function func |
+        func.getName()
             .regexpMatch("(?i).*(gen(erate)?|make|mk|create).*(nonce|salt|pepper|Password).*")
       |
-        this.getEnclosingCallable() = randomFn
+        this.asExpr().getScope() = func
       )
     }
   }

python/ql/src/meta/analysis-quality/CallGraph.ql

@@ -1,9 +1,9 @@
 /**
  * @name Call graph
- * @description An edge in the points-to call graph.
+ * @description An edge in the call graph.
  * @kind problem
  * @problem.severity recommendation
- * @id py/meta/points-to-call-graph
+ * @id py/meta/call-graph
  * @tags meta
  * @precision very-low
  */
@@ -12,9 +12,9 @@ import python
 import semmle.python.dataflow.new.internal.DataFlowPrivate
 import meta.MetaMetrics
 
-from DataFlowCall c, DataFlowCallableValue f
+from DataFlowCall call, DataFlowCallable target
 where
-  c.getCallable() = f and
-  not c.getLocation().getFile() instanceof IgnoredFile and
-  not f.getScope().getLocation().getFile() instanceof IgnoredFile
-select c, "Call to $@", f.getScope(), f.toString()
+  target = viableCallable(call) and
+  not call.getLocation().getFile() instanceof IgnoredFile and
+  not target.getScope().getLocation().getFile() instanceof IgnoredFile
+select call, "Call to $@", target.getScope(), target.toString()

python/ql/src/meta/analysis-quality/CallGraphQuality.qll

@@ -1,16 +1,55 @@
 /**
  * Provides predicates for measuring the quality of the call graph, that is,
- * the number of calls that could be resolved to a callee.
+ * the number of calls that could be resolved to a target.
  */
 
 import python
 import meta.MetaMetrics
 
+newtype TTarget =
+  TFunction(Function func) or
+  TClass(Class cls)
+
+class Target extends TTarget {
+  /** Gets a textual representation of this element. */
+  abstract string toString();
+
+  /** Gets the location of this dataflow call. */
+  abstract Location getLocation();
+
+  /** Whether this target is relevant. */
+  predicate isRelevant() { exists(this.getLocation().getFile().getRelativePath()) }
+}
+
+class TargetFunction extends Target, TFunction {
+  Function func;
+
+  TargetFunction() { this = TFunction(func) }
+
+  override string toString() { result = func.toString() }
+
+  override Location getLocation() { result = func.getLocation() }
+
+  Function getFunction() { result = func }
+}
+
+class TargetClass extends Target, TClass {
+  Class cls;
+
+  TargetClass() { this = TClass(cls) }
+
+  override string toString() { result = cls.toString() }
+
+  override Location getLocation() { result = cls.getLocation() }
+
+  Class getClass() { result = cls }
+}
+
 /**
  * A call that is (possibly) relevant for analysis quality.
  * See `IgnoredFile` for details on what is excluded.
  */
-class RelevantCall extends Call {
+class RelevantCall extends CallNode {
   RelevantCall() { not this.getLocation().getFile() instanceof IgnoredFile }
 }
 
@@ -18,12 +57,16 @@ class RelevantCall extends Call {
 module PointsToBasedCallGraph {
   /** A call that can be resolved by points-to. */
   class ResolvableCall extends RelevantCall {
-    Value callee;
+    Value targetValue;
 
-    ResolvableCall() { callee.getACall() = this.getAFlowNode() }
+    ResolvableCall() { targetValue.getACall() = this }
 
-    /** Gets a resolved callee of this call. */
-    Value getCallee() { result = callee }
+    /** Gets a resolved target of this call. */
+    Target getTarget() {
+      result.(TargetFunction).getFunction() = targetValue.(CallableValue).getScope()
+      or
+      result.(TargetClass).getClass() = targetValue.(ClassValue).getScope()
+    }
   }
 
   /** A call that cannot be resolved by points-to. */
@@ -32,34 +75,79 @@ module PointsToBasedCallGraph {
   }
 
   /**
-   * A call that can be resolved by points-to, where the resolved callee is relevant.
-   * Relevant callees include:
-   * - builtins
-   * - standard library
+   * A call that can be resolved by points-to, where the resolved target is relevant.
+   * Relevant targets include:
    * - source code of the project
    */
-  class ResolvableCallRelevantCallee extends ResolvableCall {
-    ResolvableCallRelevantCallee() {
-      callee.isBuiltin()
-      or
-      exists(File file |
-        file = callee.(CallableValue).getScope().getLocation().getFile()
-        or
-        file = callee.(ClassValue).getScope().getLocation().getFile()
-      |
-        file.inStdlib()
-        or
-        // part of the source code of the project
-        exists(file.getRelativePath())
+  class ResolvableCallRelevantTarget extends ResolvableCall {
+    ResolvableCallRelevantTarget() {
+      exists(Target target | target = this.getTarget() |
+        exists(target.getLocation().getFile().getRelativePath())
       )
     }
   }
 
   /**
-   * A call that can be resolved by points-to, where the resolved callee is not considered relevant.
-   * See `ResolvableCallRelevantCallee` for the definition of relevance.
+   * A call that can be resolved by points-to, where the resolved target is not considered relevant.
+   * See `ResolvableCallRelevantTarget` for the definition of relevance.
    */
-  class ResolvableCallIrrelevantCallee extends ResolvableCall {
-    ResolvableCallIrrelevantCallee() { not this instanceof ResolvableCallRelevantCallee }
+  class ResolvableCallIrrelevantTarget extends ResolvableCall {
+    ResolvableCallIrrelevantTarget() { not this instanceof ResolvableCallRelevantTarget }
+  }
+}
+
+/** Provides classes for call-graph resolution by using type-tracking. */
+module TypeTrackingBasedCallGraph {
+  private import semmle.python.dataflow.new.internal.DataFlowDispatch as TT
+
+  /** A call that can be resolved by type-tracking. */
+  class ResolvableCall extends RelevantCall {
+    ResolvableCall() {
+      exists(TT::TNormalCall(this, _, _))
+      or
+      TT::resolveClassCall(this, _)
+    }
+
+    /** Gets a resolved target of this call. */
+    Target getTarget() {
+      exists(TT::DataFlowCall call, TT::CallType ct, Function targetFunc |
+        call = TT::TNormalCall(this, targetFunc, ct) and
+        not ct instanceof TT::CallTypeClass and
+        targetFunc = result.(TargetFunction).getFunction()
+      )
+      or
+      // a TT::TNormalCall only exists when the call can be resolved to a function.
+      // Since points-to just says the call goes directly to the class itself, and
+      // type-tracking based wants to resolve this to the constructor, which might not
+      // exist. So to do a proper comparison, we don't require the call to be resolve to
+      // a specific function.
+      TT::resolveClassCall(this, result.(TargetClass).getClass())
+    }
+  }
+
+  /** A call that cannot be resolved by type-tracking. */
+  class UnresolvableCall extends RelevantCall {
+    UnresolvableCall() { not this instanceof ResolvableCall }
+  }
+
+  /**
+   * A call that can be resolved by type-tracking, where the resolved callee is relevant.
+   * Relevant targets include:
+   * - source code of the project
+   */
+  class ResolvableCallRelevantTarget extends ResolvableCall {
+    ResolvableCallRelevantTarget() {
+      exists(Target target | target = this.getTarget() |
+        exists(target.getLocation().getFile().getRelativePath())
+      )
+    }
+  }
+
+  /**
+   * A call that can be resolved by type-tracking, where the resolved target is not considered relevant.
+   * See `ResolvableCallRelevantTarget` for the definition of relevance.
+   */
+  class ResolvableCallIrrelevantTarget extends ResolvableCall {
+    ResolvableCallIrrelevantTarget() { not this instanceof ResolvableCallRelevantTarget }
   }
 }

python/ql/src/meta/analysis-quality/PointsToResolvableCallsRelevantTarget.ql

@@ -11,4 +11,4 @@
 import python
 import CallGraphQuality
 
-select projectRoot(), count(PointsToBasedCallGraph::ResolvableCallRelevantCallee call)
+select projectRoot(), count(PointsToBasedCallGraph::ResolvableCallRelevantTarget call)

python/ql/src/meta/analysis-quality/TTCallGraph.ql

@@ -0,0 +1,17 @@
+/**
+ * @name New call graph edge from using type-tracking instead of points-to
+ * @kind problem
+ * @problem.severity recommendation
+ * @id py/meta/type-tracking-call-graph
+ * @tags meta
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from CallNode call, Target target
+where
+  target.isRelevant() and
+  call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+select call, "$@ to $@", call, "Call", target, target.toString()

python/ql/src/meta/analysis-quality/TTCallGraphMissing.ql

@@ -0,0 +1,18 @@
+/**
+ * @name Missing call graph edge from using type-tracking instead of points-to
+ * @kind problem
+ * @problem.severity recommendation
+ * @id py/meta/call-graph-missing
+ * @tags meta
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from CallNode call, Target target
+where
+  target.isRelevant() and
+  call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+  not call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+select call, "MISSING: $@ to $@", call, "Call", target, target.toString()

python/ql/src/meta/analysis-quality/TTCallGraphNew.ql

@@ -0,0 +1,18 @@
+/**
+ * @name New call graph edge from using type-tracking instead of points-to
+ * @kind problem
+ * @problem.severity recommendation
+ * @id py/meta/call-graph-new
+ * @tags meta
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from CallNode call, Target target
+where
+  target.isRelevant() and
+  not call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+  call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+select call, "NEW: $@ to $@", call, "Call", target, target.toString()

python/ql/src/meta/analysis-quality/TTCallGraphNewAmbiguous.ql

@@ -0,0 +1,19 @@
+/**
+ * @name New call graph edge from using type-tracking instead of points-to, that is ambiguous
+ * @kind problem
+ * @problem.severity recommendation
+ * @id py/meta/call-graph-new-ambiguous
+ * @tags meta
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from CallNode call, Target target
+where
+  target.isRelevant() and
+  not call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+  call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target and
+  1 < count(call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget())
+select call, "NEW: $@ to $@", call, "Call", target, target.toString()

python/ql/src/meta/analysis-quality/TTCallGraphOverview.ql

@@ -0,0 +1,35 @@
+/**
+ * @name Call graph edge overview from using type-tracking instead of points-to
+ * @id py/meta/call-graph-overview
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from string tag, int c
+where
+  tag = "SHARED" and
+  c =
+    count(CallNode call, Target target |
+      target.isRelevant() and
+      call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+      call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+    )
+  or
+  tag = "NEW" and
+  c =
+    count(CallNode call, Target target |
+      target.isRelevant() and
+      not call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+      call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+    )
+  or
+  tag = "MISSING" and
+  c =
+    count(CallNode call, Target target |
+      target.isRelevant() and
+      call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+      not call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+    )
+select tag, c

python/ql/src/meta/analysis-quality/TTCallGraphShared.ql

@@ -0,0 +1,18 @@
+/**
+ * @name Shared call graph edge from using type-tracking instead of points-to
+ * @kind problem
+ * @problem.severity recommendation
+ * @id py/meta/call-graph-shared
+ * @tags meta
+ * @precision very-low
+ */
+
+import python
+import CallGraphQuality
+
+from CallNode call, Target target
+where
+  target.isRelevant() and
+  call.(PointsToBasedCallGraph::ResolvableCall).getTarget() = target and
+  call.(TypeTrackingBasedCallGraph::ResolvableCall).getTarget() = target
+select call, "SHARED: $@ to $@", call, "Call", target, target.toString()

python/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-queries
-version: 0.6.3-dev
+version: 0.6.5-dev
 groups: 
   - python
   - queries