Merge branch 'main' into pyMaD

python/ql/lib/CHANGELOG.md

@@ -1,3 +1,14 @@
+## 0.3.0
+
+### Breaking Changes
+
+* The imports made available from `import python` are no longer exposed under `DataFlow::` after doing `import semmle.python.dataflow.new.DataFlow`, for example using `DataFlow::Add` will now cause a compile error.
+
+### Minor Analysis Improvements
+
+* The modeling of `request.files` in Flask has been fixed, so we now properly handle assignments to local variables (such as `files = request.files; files['key'].filename`).
+* Added taint propagation for `io.StringIO` and `io.BytesIO`. This addition was originally [submitted as part of an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
+
 ## 0.2.0
 
 ### Breaking Changes

python/ql/lib/change-notes/2022-03-29-add-taint-for-StringIO.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* Added taint propagation for `io.StringIO` and `io.BytesIO`. This addition was originally [submitted as part of an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).

python/ql/lib/change-notes/2022-04-20-export-python-under-DataFlow.md

@@ -1,4 +0,0 @@
----
- category: breaking
----
- * The imports made available from `import python` are no longer exposed under `DataFlow::` after doing `import semmle.python.dataflow.new.DataFlow`, for example using `DataFlow::Add` will now cause a compile error.

python/ql/lib/change-notes/2022-05-02-flask-request-files-modeling.md

@@ -1,5 +0,0 @@
----
-category: minorAnalysis
----
-The modeling of `request.files` in Flask has been fixed, so we now properly handle
-assignments to local variables (such as `files = request.files; files['key'].filename`).

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

@@ -0,0 +1,10 @@
+## 0.3.0
+
+### Breaking Changes
+
+* The imports made available from `import python` are no longer exposed under `DataFlow::` after doing `import semmle.python.dataflow.new.DataFlow`, for example using `DataFlow::Add` will now cause a compile error.
+
+### Minor Analysis Improvements
+
+* The modeling of `request.files` in Flask has been fixed, so we now properly handle assignments to local variables (such as `files = request.files; files['key'].filename`).
+* Added taint propagation for `io.StringIO` and `io.BytesIO`. This addition was originally [submitted as part of an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.2.0
+lastReleaseVersion: 0.3.0

python/ql/lib/qlpack.yml

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

python/ql/lib/semmle/python/Import.qll

@@ -184,7 +184,7 @@ class Import extends Import_ {
    * For example, for the import statement `import bar` which
    * is a relative import in package "foo", this would return
    * "foo.bar".
-   * The import statment `from foo import bar` would return
+   * The import statement `from foo import bar` would return
    * `foo` and `foo.bar`
    */
   string getAnImportedModuleName() {

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

@@ -172,7 +172,7 @@ module ArgumentPassing {
   /**
    * Gets the node representing the argument to `call` that is passed to the parameter at
    * (zero-based) index `paramN` in `callable`. If this is a positional argument, it must appear
-   * at an index, `argN`, in `call` wich satisfies `paramN = mapping.getParamN(argN)`.
+   * at an index, `argN`, in `call` which satisfies `paramN = mapping.getParamN(argN)`.
    *
    * `mapping` will be the identity for function calls, but not for method- or constructor calls,
    * where the first parameter is `self` and the first positional argument is passed to the second positional parameter.

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

@@ -170,6 +170,14 @@ abstract class Configuration extends string {
    */
   int explorationLimit() { none() }
 
+  /**
+   * Holds if hidden nodes should be included in the data flow graph.
+   *
+   * This feature should only be used for debugging or when the data flow graph
+   * is not visualized (for example in a `path-problem` query).
+   */
+  predicate includeHiddenNodes() { none() }
+
   /**
    * Holds if there is a partial data flow path from `source` to `node`. The
    * approximate distance between `node` and the closest source is `dist` and
@@ -3653,7 +3661,7 @@ private newtype TPathNode =
  * of dereference operations needed to get from the value in the node to the
  * tracked object. The final type indicates the type of the tracked object.
  */
-abstract private class AccessPath extends TAccessPath {
+private class AccessPath extends TAccessPath {
   /** Gets the head of this access path, if any. */
   abstract TypedContent getHead();
 
@@ -3868,11 +3876,14 @@ abstract private class PathNodeImpl extends PathNode {
   abstract NodeEx getNodeEx();
 
   predicate isHidden() {
-    hiddenNode(this.getNodeEx().asNode()) and
-    not this.isSource() and
-    not this instanceof PathNodeSink
-    or
-    this.getNodeEx() instanceof TNodeImplicitRead
+    not this.getConfiguration().includeHiddenNodes() and
+    (
+      hiddenNode(this.getNodeEx().asNode()) and
+      not this.isSource() and
+      not this instanceof PathNodeSink
+      or
+      this.getNodeEx() instanceof TNodeImplicitRead
+    )
   }
 
   private string ppAp() {

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

@@ -170,6 +170,14 @@ abstract class Configuration extends string {
    */
   int explorationLimit() { none() }
 
+  /**
+   * Holds if hidden nodes should be included in the data flow graph.
+   *
+   * This feature should only be used for debugging or when the data flow graph
+   * is not visualized (for example in a `path-problem` query).
+   */
+  predicate includeHiddenNodes() { none() }
+
   /**
    * Holds if there is a partial data flow path from `source` to `node`. The
    * approximate distance between `node` and the closest source is `dist` and
@@ -3653,7 +3661,7 @@ private newtype TPathNode =
  * of dereference operations needed to get from the value in the node to the
  * tracked object. The final type indicates the type of the tracked object.
  */
-abstract private class AccessPath extends TAccessPath {
+private class AccessPath extends TAccessPath {
   /** Gets the head of this access path, if any. */
   abstract TypedContent getHead();
 
@@ -3868,11 +3876,14 @@ abstract private class PathNodeImpl extends PathNode {
   abstract NodeEx getNodeEx();
 
   predicate isHidden() {
-    hiddenNode(this.getNodeEx().asNode()) and
-    not this.isSource() and
-    not this instanceof PathNodeSink
-    or
-    this.getNodeEx() instanceof TNodeImplicitRead
+    not this.getConfiguration().includeHiddenNodes() and
+    (
+      hiddenNode(this.getNodeEx().asNode()) and
+      not this.isSource() and
+      not this instanceof PathNodeSink
+      or
+      this.getNodeEx() instanceof TNodeImplicitRead
+    )
   }
 
   private string ppAp() {

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

@@ -170,6 +170,14 @@ abstract class Configuration extends string {
    */
   int explorationLimit() { none() }
 
+  /**
+   * Holds if hidden nodes should be included in the data flow graph.
+   *
+   * This feature should only be used for debugging or when the data flow graph
+   * is not visualized (for example in a `path-problem` query).
+   */
+  predicate includeHiddenNodes() { none() }
+
   /**
    * Holds if there is a partial data flow path from `source` to `node`. The
    * approximate distance between `node` and the closest source is `dist` and
@@ -3653,7 +3661,7 @@ private newtype TPathNode =
  * of dereference operations needed to get from the value in the node to the
  * tracked object. The final type indicates the type of the tracked object.
  */
-abstract private class AccessPath extends TAccessPath {
+private class AccessPath extends TAccessPath {
   /** Gets the head of this access path, if any. */
   abstract TypedContent getHead();
 
@@ -3868,11 +3876,14 @@ abstract private class PathNodeImpl extends PathNode {
   abstract NodeEx getNodeEx();
 
   predicate isHidden() {
-    hiddenNode(this.getNodeEx().asNode()) and
-    not this.isSource() and
-    not this instanceof PathNodeSink
-    or
-    this.getNodeEx() instanceof TNodeImplicitRead
+    not this.getConfiguration().includeHiddenNodes() and
+    (
+      hiddenNode(this.getNodeEx().asNode()) and
+      not this.isSource() and
+      not this instanceof PathNodeSink
+      or
+      this.getNodeEx() instanceof TNodeImplicitRead
+    )
   }
 
   private string ppAp() {

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

@@ -170,6 +170,14 @@ abstract class Configuration extends string {
    */
   int explorationLimit() { none() }
 
+  /**
+   * Holds if hidden nodes should be included in the data flow graph.
+   *
+   * This feature should only be used for debugging or when the data flow graph
+   * is not visualized (for example in a `path-problem` query).
+   */
+  predicate includeHiddenNodes() { none() }
+
   /**
    * Holds if there is a partial data flow path from `source` to `node`. The
    * approximate distance between `node` and the closest source is `dist` and
@@ -3653,7 +3661,7 @@ private newtype TPathNode =
  * of dereference operations needed to get from the value in the node to the
  * tracked object. The final type indicates the type of the tracked object.
  */
-abstract private class AccessPath extends TAccessPath {
+private class AccessPath extends TAccessPath {
   /** Gets the head of this access path, if any. */
   abstract TypedContent getHead();
 
@@ -3868,11 +3876,14 @@ abstract private class PathNodeImpl extends PathNode {
   abstract NodeEx getNodeEx();
 
   predicate isHidden() {
-    hiddenNode(this.getNodeEx().asNode()) and
-    not this.isSource() and
-    not this instanceof PathNodeSink
-    or
-    this.getNodeEx() instanceof TNodeImplicitRead
+    not this.getConfiguration().includeHiddenNodes() and
+    (
+      hiddenNode(this.getNodeEx().asNode()) and
+      not this.isSource() and
+      not this instanceof PathNodeSink
+      or
+      this.getNodeEx() instanceof TNodeImplicitRead
+    )
   }
 
   private string ppAp() {

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

@@ -305,7 +305,7 @@ cached
 private module Cached {
   /**
    * If needed, call this predicate from `DataFlowImplSpecific.qll` in order to
-   * force a stage-dependency on the `DataFlowImplCommon.qll` stage and therby
+   * force a stage-dependency on the `DataFlowImplCommon.qll` stage and thereby
    * collapsing the two stages.
    */
   cached

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

@@ -45,14 +45,14 @@ abstract class TrackableState extends string {
   /**
    * Holds if state starts at `f`.
    * Either this predicate or `startsAt(ControlFlowNode f, Context ctx)`
-   * should be overriden by sub-classes.
+   * should be overridden by sub-classes.
    */
   predicate startsAt(ControlFlowNode f) { none() }
 
   /**
    * Holds if state starts at `f` given context `ctx`.
    * Either this predicate or `startsAt(ControlFlowNode f)`
-   * should be overriden by sub-classes.
+   * should be overridden by sub-classes.
    */
   pragma[noinline]
   predicate startsAt(ControlFlowNode f, Context ctx) { ctx.appliesTo(f) and this.startsAt(f) }
@@ -60,14 +60,14 @@ abstract class TrackableState extends string {
   /**
    * Holds if state ends at `f`.
    * Either this predicate or `endsAt(ControlFlowNode f, Context ctx)`
-   * may be overriden by sub-classes.
+   * may be overridden by sub-classes.
    */
   predicate endsAt(ControlFlowNode f) { none() }
 
   /**
    * Holds if state ends at `f` given context `ctx`.
    * Either this predicate or `endsAt(ControlFlowNode f)`
-   * may be overriden by sub-classes.
+   * may be overridden by sub-classes.
    */
   pragma[noinline]
   predicate endsAt(ControlFlowNode f, Context ctx) { ctx.appliesTo(f) and this.endsAt(f) }

python/ql/lib/semmle/python/frameworks/Aiomysql.qll

@@ -25,7 +25,7 @@ private module Aiomysql {
   /**
    * Gets a `Connection` that is created when
    * - the result of `aiomysql.connect()` is awaited.
-   * - the result of calling `aquire` on a `ConnectionPool` is awaited.
+   * - the result of calling `acquire` on a `ConnectionPool` is awaited.
    * See https://aiomysql.readthedocs.io/en/stable/connection.html#connection
    */
   API::Node connection() {
@@ -82,7 +82,7 @@ private module Aiomysql {
   }
 
   /**
-   * Gets an `SAConnection` that is created when the result of calling `aquire` on an `Engine` is awaited.
+   * Gets an `SAConnection` that is created when the result of calling `acquire` on an `Engine` is awaited.
    * See https://aiomysql.readthedocs.io/en/stable/sa.html#connection
    */
   API::Node saConnection() { result = engine().getMember("acquire").getReturn().getAwaited() }

python/ql/lib/semmle/python/frameworks/Aiopg.qll

@@ -25,7 +25,7 @@ private module Aiopg {
   /**
    * Gets a `Connection` that is created when
    * - the result of `aiopg.connect()` is awaited.
-   * - the result of calling `aquire` on a `ConnectionPool` is awaited.
+   * - the result of calling `acquire` on a `ConnectionPool` is awaited.
    * See https://aiopg.readthedocs.io/en/stable/core.html#connection
    */
   API::Node connection() {
@@ -78,7 +78,7 @@ private module Aiopg {
   }
 
   /**
-   * Gets an `SAConnection` that is created when the result of calling `aquire` on an `Engine` is awaited.
+   * Gets an `SAConnection` that is created when the result of calling `acquire` on an `Engine` is awaited.
    * See https://aiopg.readthedocs.io/en/stable/sa.html#connection
    */
   API::Node saConnection() { result = engine().getMember("acquire").getReturn().getAwaited() }

python/ql/lib/semmle/python/frameworks/Asyncpg.qll

@@ -20,7 +20,7 @@ private module Asyncpg {
           "asyncpg;ConnectionPool;asyncpg;;Member[create_pool].ReturnValue.Awaited",
           // a `Connection` that is created when
           // * - the result of `asyncpg.connect()` is awaited.
-          // * - the result of calling `aquire` on a `ConnectionPool` is awaited.
+          // * - the result of calling `acquire` on a `ConnectionPool` is awaited.
           "asyncpg;Connection;asyncpg;;Member[connect].ReturnValue.Awaited",
           "asyncpg;Connection;asyncpg;ConnectionPool;Member[acquire].ReturnValue.Awaited",
           // Creating an internal `~Connection` type that contains both `Connection` and `ConnectionPool`.

python/ql/lib/semmle/python/objects/TObject.qll

@@ -243,7 +243,7 @@ predicate class_method(
  * Holds if the literal corresponding to the control flow node `n` has class `cls`.
  *
  * Helper predicate for `literal_instantiation`. Prevents a bad join with
- * `PointsToContext::appliesTo` from occuring.
+ * `PointsToContext::appliesTo` from occurring.
  */
 pragma[nomagic]
 private predicate literal_node_class(ControlFlowNode n, ClassObjectInternal cls) {

python/ql/lib/semmle/python/security/BadTagFilterQuery.qll

@@ -28,14 +28,14 @@ private module RegexpMatching {
      * but if `ignorePrefix` is true, it will only match "foo".
      */
     predicate test(string str, boolean ignorePrefix) {
-      none() // maybe overriden in subclasses
+      none() // maybe overridden in subclasses
     }
 
     /**
      * Same as `test(..)`, but where the `fillsCaptureGroup` afterwards tells which capture groups were filled by the given string.
      */
     predicate testWithGroups(string str, boolean ignorePrefix) {
-      none() // maybe overriden in subclasses
+      none() // maybe overridden in subclasses
     }
 
     /**

python/ql/lib/semmle/python/security/performance/ExponentialBackTracking.qll

@@ -51,7 +51,7 @@
  *     either a single character, a set of characters represented by a
  *     character class, or the set of all characters.
  *   * The product automaton is constructed lazily, starting with pair states
- *     `(q, q)` where `q` is a fork, and proceding along an over-approximate
+ *     `(q, q)` where `q` is a fork, and proceeding along an over-approximate
  *     step relation.
  *   * The over-approximate step relation allows transitions along pairs of
  *     abstract input symbols where the symbols have overlap in the characters they accept.

python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll

@@ -610,16 +610,23 @@ State after(RegExpTerm t) {
   or
   exists(RegExpGroup grp | t = grp.getAChild() | result = after(grp))
   or
-  exists(EffectivelyStar star | t = star.getAChild() | result = before(star))
+  exists(EffectivelyStar star | t = star.getAChild() |
+    not isPossessive(star) and
+    result = before(star)
+  )
   or
   exists(EffectivelyPlus plus | t = plus.getAChild() |
-    result = before(plus) or
+    not isPossessive(plus) and
+    result = before(plus)
+    or
     result = after(plus)
   )
   or
   exists(EffectivelyQuestion opt | t = opt.getAChild() | result = after(opt))
   or
-  exists(RegExpRoot root | t = root | result = AcceptAnySuffix(root))
+  exists(RegExpRoot root | t = root |
+    if matchesAnySuffix(root) then result = AcceptAnySuffix(root) else result = Accept(root)
+  )
 }
 
 /**
@@ -690,7 +697,7 @@ predicate delta(State q1, EdgeLabel lbl, State q2) {
     lbl = Epsilon() and q2 = Accept(root)
   )
   or
-  exists(RegExpRoot root | q1 = Match(root, 0) | lbl = Any() and q2 = q1)
+  exists(RegExpRoot root | q1 = Match(root, 0) | matchesAnyPrefix(root) and lbl = Any() and q2 = q1)
   or
   exists(RegExpDollar dollar | q1 = before(dollar) |
     lbl = Epsilon() and q2 = Accept(getRoot(dollar))

python/ql/lib/semmle/python/security/performance/ReDoSUtilSpecific.qll

@@ -13,6 +13,24 @@ predicate isEscapeClass(RegExpTerm term, string clazz) {
   exists(RegExpCharacterClassEscape escape | term = escape | escape.getValue() = clazz)
 }
 
+/**
+ * Holds if `term` is a possessive quantifier.
+ * As python's regexes do not support possessive quantifiers, this never holds, but is used by the shared library.
+ */
+predicate isPossessive(RegExpQuantifier term) { none() }
+
+/**
+ * Holds if the regex that `term` is part of is used in a way that ignores any leading prefix of the input it's matched against.
+ * Not yet implemented for Python.
+ */
+predicate matchesAnyPrefix(RegExpTerm term) { any() }
+
+/**
+ * Holds if the regex that `term` is part of is used in a way that ignores any trailing suffix of the input it's matched against.
+ * Not yet implemented for Python.
+ */
+predicate matchesAnySuffix(RegExpTerm term) { any() }
+
 /**
  * Holds if the regular expression should not be considered.
  *

python/ql/lib/semmle/python/security/strings/External.qll

@@ -35,7 +35,7 @@ deprecated class ExternalStringSequenceKind extends SequenceKind {
 }
 
 /**
- * An hierachical dictionary or list where the entire structure is externally controlled
+ * An hierarchical dictionary or list where the entire structure is externally controlled
  * This is typically a parsed JSON object.
  */
 deprecated class ExternalJsonKind extends TaintKind {

python/ql/src/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 0.1.2
+
+### New Queries
+
+* "XML external entity expansion" (`py/xxe`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
+* "XML internal entity expansion" (`py/xml-bomb`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
+* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.
+
 ## 0.1.1
 
 ## 0.1.0

python/ql/src/change-notes/2022-03-24-csrf-protection.md

@@ -1,4 +0,0 @@
----
- category: newQuery
----
-* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.

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

@@ -1,5 +1,7 @@
----
-category: newQuery
----
+## 0.1.2
+
+### New Queries
+
 * "XML external entity expansion" (`py/xxe`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
 * "XML internal entity expansion" (`py/xml-bomb`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
+* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.1.1
+lastReleaseVersion: 0.1.2

python/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/python-queries
-version: 0.1.2-dev
+version: 0.1.3-dev
 groups: 
   - python
   - queries

python/ql/test/TestUtilities/InlineExpectationsTest.qll

@@ -181,7 +181,7 @@ private string expectationCommentPattern() { result = "\\s*\\$((?:[^/]|/[^/])*)(
 /**
  * The possible columns in an expectation comment. The `TDefaultColumn` branch represents the first
  * column in a comment. This column is not precedeeded by a name. `TNamedColumn(name)` represents a
- * column containing expected results preceeded by the string `name:`.
+ * column containing expected results preceded by the string `name:`.
  */
 private newtype TColumn =
   TDefaultColumn() or

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

@@ -8,7 +8,7 @@
  *      s = SOURCE
  *      SINK(s)
  * ```
- * `SOURCE` will be a source and the second occurance of `s` will be a sink.
+ * `SOURCE` will be a source and the second occurrence of `s` will be a sink.
  *
  * In order to test literals, alternative sources are defined for each type:
  *