Merge branch 'main' into python-dataflow/flow-summaries-from-scratch

python/ql/lib/CHANGELOG.md

@@ -1,3 +1,20 @@
+## 0.5.2
+
+## 0.5.1
+
+### Deprecated APIs
+
+- The documentation of API graphs (the `API` module) has been expanded, and some of the members predicates of `API::Node`
+  have been renamed as follows:
+  - `getAnImmediateUse` -> `asSource`
+  - `getARhs` -> `asSink`
+  - `getAUse` -> `getAValueReachableFromSource`
+  - `getAValueReachingRhs` -> `getAValueReachingSink`
+
+### Minor Analysis Improvements
+
+* Improved modeling of sensitive data sources, so common words like `certain` and `secretary` are no longer considered a certificate and a secret (respectively).
+
 ## 0.5.0
 
 ### Deprecated APIs

python/ql/lib/analysis/LocalDefinitions.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Jump-to-definition links
+ * @description Generates use-definition pairs that provide the data
+ *              for jump-to-definition in the code viewer.
+ * @kind definitions
+ * @id py/ide-jump-to-definition
+ * @tags ide-contextual-queries/local-definitions
+ */
+
+import python
+import analysis.DefinitionTracking
+
+external string selectedSourceFile();
+
+from NiceLocationExpr use, Definition defn, string kind, string f
+where
+  defn = definitionOf(use, kind) and
+  use.hasLocationInfo(f, _, _, _, _) and
+  getFileBySourceArchiveName(selectedSourceFile()).getAbsolutePath() = f
+select use, defn, kind

python/ql/lib/analysis/LocalReferences.ql

@@ -0,0 +1,19 @@
+/**
+ * @name Find-references links
+ * @description Generates use-definition pairs that provide the data
+ *              for find-references in the code viewer.
+ * @kind definitions
+ * @id py/ide-find-references
+ * @tags ide-contextual-queries/local-references
+ */
+
+import python
+import analysis.DefinitionTracking
+
+external string selectedSourceFile();
+
+from NiceLocationExpr use, Definition defn, string kind
+where
+  defn = definitionOf(use, kind) and
+  defn.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile())
+select use, defn, kind

python/ql/lib/change-notes/2022-06-15-class-decorator-api-subclass.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Change `.getASubclass()` on `API::Node` so it allows to follow subclasses even if the class has a class decorator.

python/ql/lib/change-notes/2022-06-22-sensitive-common-words.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Improved modeling of sensitive data sources, so common words like `certain` and `secretary` are no longer considered a certificate and a secret (respectively).

python/ql/lib/change-notes/released/0.5.1.md

@@ -1,6 +1,6 @@
----
-category: deprecated
----
+## 0.5.1
+
+### Deprecated APIs
 
 - The documentation of API graphs (the `API` module) has been expanded, and some of the members predicates of `API::Node`
   have been renamed as follows:
@@ -8,3 +8,7 @@ category: deprecated
   - `getARhs` -> `asSink`
   - `getAUse` -> `getAValueReachableFromSource`
   - `getAValueReachingRhs` -> `getAValueReachingSink`
+
+### Minor Analysis Improvements
+
+* Improved modeling of sensitive data sources, so common words like `certain` and `secretary` are no longer considered a certificate and a secret (respectively).

python/ql/lib/change-notes/released/0.5.2.md

@@ -0,0 +1 @@
+## 0.5.2

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.5.0
+lastReleaseVersion: 0.5.2

python/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-all
-version: 0.5.1-dev
+version: 0.5.3-dev
 groups: python
 dbscheme: semmlecode.python.dbscheme
 extractor: python

python/ql/lib/semmle/python/ApiGraphs.qll

@@ -719,9 +719,16 @@ module API {
         or
         // Subclassing a node
         lbl = Label::subclass() and
-        exists(DataFlow::Node superclass | pred.flowsTo(superclass) |
-          ref.(DataFlow::ExprNode).getNode().getNode().(PY::ClassExpr).getABase() =
-            superclass.(DataFlow::ExprNode).getNode().getNode()
+        exists(PY::ClassExpr clsExpr, DataFlow::Node superclass | pred.flowsTo(superclass) |
+          clsExpr.getABase() = superclass.asExpr() and
+          // Potentially a class decorator could do anything, but we assume they are
+          // "benign" and let subclasses edges flow through anyway.
+          // see example in https://github.com/django/django/blob/c2250cfb80e27cdf8d098428824da2800a18cadf/tests/auth_tests/test_views.py#L40-L46
+          (
+            ref.(DataFlow::ExprNode).getNode().getNode() = clsExpr
+            or
+            ref.(DataFlow::ExprNode).getNode().getNode() = clsExpr.getADecoratorCall()
+          )
         )
         or
         // awaiting

python/ql/lib/semmle/python/Frameworks.qll

@@ -2,7 +2,7 @@
  * Helper file that imports all framework modeling.
  */
 
-// If you add modeling of a new framework/library, remember to add it it to the docs in
+// If you add modeling of a new framework/library, remember to add it to the docs in
 // `docs/codeql/support/reusables/frameworks.rst`
 private import semmle.python.frameworks.Aioch
 private import semmle.python.frameworks.Aiohttp

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

@@ -554,16 +554,55 @@ class StarPatternElementNode extends Node, TStarPatternElementNode {
   override Location getLocation() { result = consumer.getLocation() }
 }
 
+/**
+ * Gets a node that controls whether other nodes are evaluated.
+ *
+ * In the base case, this is the last node of `conditionBlock`, and `flipped` is `false`.
+ * This definition accounts for (short circuting) `and`- and `or`-expressions, as the structure
+ * of basic blocks will reflect their semantics.
+ *
+ * However, in the program
+ * ```python
+ * if not is_safe(path):
+ *   return
+ * ```
+ * the last node in the `ConditionBlock` is `not is_safe(path)`.
+ *
+ * We would like to consider also `is_safe(path)` a guard node, albeit with `flipped` being `true`.
+ * Thus we recurse through `not`-expressions.
+ */
+ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) {
+  // Base case: the last node truly does determine which successor is chosen
+  result = conditionBlock.getLastNode() and
+  flipped = false
+  or
+  // Recursive case: if a guard node is a `not`-expression,
+  // the operand is also a guard node, but with inverted polarity.
+  exists(UnaryExprNode notNode |
+    result = notNode.getOperand() and
+    notNode.getNode().getOp() instanceof Not
+  |
+    notNode = guardNode(conditionBlock, flipped.booleanNot())
+  )
+}
+
 /**
  * A node that controls whether other nodes are evaluated.
+ *
+ * The field `flipped` allows us to match `GuardNode`s underneath
+ * `not`-expressions and still choose the appropriate branch.
  */
 class GuardNode extends ControlFlowNode {
   ConditionBlock conditionBlock;
+  boolean flipped;
 
-  GuardNode() { this = conditionBlock.getLastNode() }
+  GuardNode() { this = guardNode(conditionBlock, flipped) }
 
   /** Holds if this guard controls block `b` upon evaluating to `branch`. */
-  predicate controlsBlock(BasicBlock b, boolean branch) { conditionBlock.controls(b, branch) }
+  predicate controlsBlock(BasicBlock b, boolean branch) {
+    branch in [true, false] and
+    conditionBlock.controls(b, branch.booleanXor(flipped))
+  }
 }
 
 /**

python/ql/lib/semmle/python/dataflow/old/TaintTracking.qll

@@ -333,7 +333,7 @@ abstract class Sanitizer extends string {
   /** Holds if `taint` cannot flow through `node`. */
   predicate sanitizingNode(TaintKind taint, ControlFlowNode node) { none() }
 
-  /** Holds if `call` removes removes the `taint` */
+  /** Holds if `call` removes the `taint` */
   predicate sanitizingCall(TaintKind taint, FunctionObject callee) { none() }
 
   /** Holds if `test` shows value to be untainted with `taint` */

python/ql/lib/semmle/python/frameworks/internal/SubclassFinder.qll

@@ -74,9 +74,7 @@ private module NotExposed {
   }
 
   /** DEPRECATED: Alias for fullyQualifiedToApiGraphPath */
-  deprecated string fullyQualifiedToAPIGraphPath(string fullyQaulified) {
-    result = fullyQualifiedToApiGraphPath(fullyQaulified)
-  }
+  deprecated predicate fullyQualifiedToAPIGraphPath = fullyQualifiedToApiGraphPath/1;
 
   bindingset[this]
   abstract class FindSubclassesSpec extends string {

python/ql/lib/semmle/python/pointsto/PointsToContext.qll

@@ -23,13 +23,8 @@ private int max_context_cost() {
 }
 
 private int syntactic_call_count(Scope s) {
-  exists(Function f | f = s and f.getName() != "__init__" |
-    result =
-      count(CallNode call |
-        call.getFunction().(NameNode).getId() = f.getName()
-        or
-        call.getFunction().(AttrNode).getName() = f.getName()
-      )
+  exists(Function f, string name | f = s and name = f.getName() and name != "__init__" |
+    result = count(function_call(name)) + count(method_call(name))
   )
   or
   s.getName() = "__init__" and result = 1
@@ -37,6 +32,12 @@ private int syntactic_call_count(Scope s) {
   not s instanceof Function and result = 0
 }
 
+pragma[nomagic]
+private CallNode function_call(string name) { result.getFunction().(NameNode).getId() = name }
+
+pragma[nomagic]
+private CallNode method_call(string name) { result.getFunction().(AttrNode).getName() = name }
+
 private int incoming_call_cost(Scope s) {
   /*
    * Syntactic call count will often be a considerable overestimate