Revert "JS: Recognize DomSanitizer from @angular/core"

This reverts commit ff1d0cc4c7.

CONTRIBUTING.md

@@ -36,7 +36,7 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     For details, see the [guide on query metadata](docs/query-metadata-style-guide.md).
 
-    Make sure the `select` statement is compatible with the query `@kind`. See [About CodeQL queries](https://help.semmle.com/QL/learn-ql/writing-queries/introduction-to-queries.html#select-clause) on help.semmle.com.
+    Make sure the `select` statement is compatible with the query `@kind`. See [About CodeQL queries](https://codeql.github.com/docs/writing-codeql-queries/about-codeql-queries/#select-clause) on codeql.github.com.
 
 3. **Formatting**
 

benjamin-button.md

@@ -0,0 +1,51 @@
+# benjamin-buttons.md
+
+This file describes the changes that have been applied to
+the library to make it behave as if it was younger.
+
+## TaintedPath.ql
+
+Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
+
+- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
+
+Sinks added between 2018-08-02 and 2020-01-01 have been removed. Found by looking at:
+
+- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+pathinjection
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+tainted-path
+
+Sinks from the "graceful-fs" and "fs-extra" (added before the open-sourcing squash).
+
+## Xss.ql
+
+Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
+
+- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
+
+- recursive type tracking for `jQuery::dollar`, `DOM::domValueRef`.
+
+## SqlInjection.ql
+
+Sinks added between 2020-01-01 and 2020-10-06 have been removed. Found by looking at:
+
+- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-089
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
+
+Sinks added between 2018-08-02 and 2020-01-01 have been removed. Found by looking at:
+
+- the commit titles of https://github.com/github/codeql/commits/main/javascript/ql/test/query-tests/Security/CWE-089
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sink
+- the PR titles of https://github.com/github/codeql/pulls?page=2&q=is%3Apr+label%3AJS+is%3Aclosed+sql
+
+TypeTracking in SQL.qll (added before the open-sourcing squash)
+
+The model of `mssql` and `sequelize` (added before the open-sourcing squash)
+
+## PseudoProperties
+
+Pseudo-properties (`$name$`) used in type-tracking and global dataflow configurations have been disabled.
+Found by searching for `"\$.*\$"`.

config/identical-files.json

@@ -482,11 +482,12 @@
     "python/ql/lib/semmle/python/security/performance/ReDoSUtil.qll",
     "ruby/ql/lib/codeql/ruby/security/performance/ReDoSUtil.qll"
   ],
-  "ReDoS Exponential Python/JS": [
+  "ReDoS Exponential Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/performance/ExponentialBackTracking.qll",
-    "python/ql/lib/semmle/python/security/performance/ExponentialBackTracking.qll"
+    "python/ql/lib/semmle/python/security/performance/ExponentialBackTracking.qll",
+    "ruby/ql/lib/codeql/ruby/security/performance/ExponentialBackTracking.qll"
   ],
-  "ReDoS Polynomial Python/JS": [
+  "ReDoS Polynomial Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/performance/SuperlinearBackTracking.qll",
     "python/ql/lib/semmle/python/security/performance/SuperlinearBackTracking.qll",
     "ruby/ql/lib/codeql/ruby/security/performance/SuperlinearBackTracking.qll"
@@ -520,10 +521,27 @@
   ],
   "Hostname Regexp queries": [
     "javascript/ql/src/Security/CWE-020/HostnameRegexpShared.qll",
+    "python/ql/src/Security/CWE-020/HostnameRegexpShared.qll",
     "ruby/ql/src/queries/security/cwe-020/HostnameRegexpShared.qll"
   ],
   "ApiGraphModels": [
     "javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll",
     "ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll"
+  ],
+  "TaintedFormatStringQuery Ruby/JS": [
+    "javascript/ql/lib/semmle/javascript/security/dataflow/TaintedFormatStringQuery.qll",
+    "ruby/ql/lib/codeql/ruby/security/TaintedFormatStringQuery.qll"
+  ],
+  "TaintedFormatStringCustomizations Ruby/JS": [
+    "javascript/ql/lib/semmle/javascript/security/dataflow/TaintedFormatStringCustomizations.qll",
+    "ruby/ql/lib/codeql/ruby/security/TaintedFormatStringCustomizations.qll"
+  ],
+  "HttpToFileAccessQuery JS/Ruby": [
+    "javascript/ql/lib/semmle/javascript/security/dataflow/HttpToFileAccessQuery.qll",
+    "ruby/ql/lib/codeql/ruby/security/HttpToFileAccessQuery.qll"
+  ],
+  "HttpToFileAccessCustomizations JS/Ruby": [
+    "javascript/ql/lib/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll",
+    "ruby/ql/lib/codeql/ruby/security/HttpToFileAccessCustomizations.qll"
   ]
-}
+}

cpp/ql/lib/CHANGELOG.md

@@ -1,26 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
-* `DefaultOptions::exits` now holds for C11 functions with the `_Noreturn` or `noreturn` specifier.
-* `hasImplicitCopyConstructor` and `hasImplicitCopyAssignmentOperator` now correctly handle implicitly-deleted operators in templates.
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.
-
 ## 0.0.11
 
 ### Minor Analysis Improvements

cpp/ql/lib/change-notes/2022-02-07-deleted-deprecations.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

cpp/ql/lib/change-notes/2022-02-07-deprecated-acronyms.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.

cpp/ql/lib/change-notes/2022-03-10-template-implicit-copy.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `hasImplicitCopyConstructor` and `hasImplicitCopyAssignmentOperator` now correctly handle implicitly-deleted operators in templates.

cpp/ql/lib/change-notes/2022-03-14-c11-noreturn.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* `DefaultOptions::exits` now holds for C11 functions with the `_Noreturn` or `noreturn` specifier.

cpp/ql/lib/change-notes/2022-03-14-flow-state-barriers.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.

cpp/ql/lib/change-notes/2022-03-14-taint-interface-cleanup.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.

cpp/ql/lib/change-notes/released/0.0.12.md

@@ -1,20 +0,0 @@
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
-* `DefaultOptions::exits` now holds for C11 functions with the `_Noreturn` or `noreturn` specifier.
-* `hasImplicitCopyConstructor` and `hasImplicitCopyAssignmentOperator` now correctly handle implicitly-deleted operators in templates.
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

cpp/ql/lib/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.0.13
+version: 0.0.12-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

cpp/ql/lib/semmle/code/cpp/Type.qll

@@ -94,6 +94,7 @@ class Type extends Locatable, @type {
    * The result of this predicate will be the type itself, except in the case of a TypedefType or a Decltype,
    * in which case the result will be type which results from (possibly recursively) resolving typedefs.
    */
+  pragma[nomagic]
   Type getUnderlyingType() { result = this }
 
   /**

cpp/ql/lib/semmle/code/cpp/controlflow/StackVariableReachability.qll

@@ -80,11 +80,7 @@ abstract class StackVariableReachability extends string {
         j > i and
         sink = bb.getNode(j) and
         this.isSink(sink, v) and
-        not exists(int k, ControlFlowNode node |
-          node = bb.getNode(k) and this.isBarrier(pragma[only_bind_into](node), v)
-        |
-          k in [i + 1 .. j - 1]
-        )
+        not exists(int k | this.isBarrier(bb.getNode(k), v) | k in [i + 1 .. j - 1])
       )
       or
       not exists(int k | this.isBarrier(bb.getNode(k), v) | k > i) and

cpp/ql/lib/semmle/code/cpp/security/Overflow.qll

@@ -25,7 +25,6 @@ predicate guardedAbs(Operation e, Expr use) {
  * Holds if the value of `use` is guarded to be less than something, and `e`
  * is in code controlled by that guard (where the guard condition held).
  */
-pragma[nomagic]
 predicate guardedLesser(Operation e, Expr use) {
   exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), true))
   or
@@ -36,7 +35,6 @@ predicate guardedLesser(Operation e, Expr use) {
  * Holds if the value of `use` is guarded to be greater than something, and `e`
  * is in code controlled by that guard (where the guard condition held).
  */
-pragma[nomagic]
 predicate guardedGreater(Operation e, Expr use) {
   exists(GuardCondition c | c.ensuresLt(use, _, _, e.getBasicBlock(), false))
   or

cpp/ql/src/CHANGELOG.md

@@ -1,12 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
-### Minor Analysis Improvements
-
-* The `cpp/overflow-destination`, `cpp/unclear-array-index-validation`, and `cpp/uncontrolled-allocation-size` queries have been modernized and converted to `path-problem` queries and provide more true positive results.
-* The `cpp/system-data-exposure` query has been increased from `medium` to `high` precision, following a number of improvements to the query logic.
-
 ## 0.0.11
 
 ### Breaking Changes

cpp/ql/src/change-notes/2022-03-07-system-data-exposure.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cpp/system-data-exposure` query has been increased from `medium` to `high` precision, following a number of improvements to the query logic.

cpp/ql/src/change-notes/2022-03-10-port-three-queries-to-taint-tracking.md

@@ -1,6 +1,4 @@
-## 0.0.12
-
-### Minor Analysis Improvements
-
+---
+category: minorAnalysis
+---
 * The `cpp/overflow-destination`, `cpp/unclear-array-index-validation`, and `cpp/uncontrolled-allocation-size` queries have been modernized and converted to `path-problem` queries and provide more true positive results.
-* The `cpp/system-data-exposure` query has been increased from `medium` to `high` precision, following a number of improvements to the query logic.

cpp/ql/src/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 0.0.13
+version: 0.0.12-dev
 groups: 
   - cpp
   - queries

csharp/ql/campaigns/Solorigate/lib/CHANGELOG.md

@@ -1,7 +1,3 @@
-## 1.0.7
-
-## 1.0.6
-
 ## 1.0.5
 
 ## 1.0.4

csharp/ql/campaigns/Solorigate/lib/change-notes/released/1.0.6.md

@@ -1 +0,0 @@
-## 1.0.6

csharp/ql/campaigns/Solorigate/lib/change-notes/released/1.0.7.md

@@ -1 +0,0 @@
-## 1.0.7

csharp/ql/campaigns/Solorigate/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.0.7
+lastReleaseVersion: 1.0.5

csharp/ql/campaigns/Solorigate/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/csharp-solorigate-all
-version: 1.0.7
+version: 1.0.6-dev
 groups:
     - csharp
     - solorigate

csharp/ql/campaigns/Solorigate/src/CHANGELOG.md

@@ -1,7 +1,3 @@
-## 1.0.7
-
-## 1.0.6
-
 ## 1.0.5
 
 ## 1.0.4

csharp/ql/campaigns/Solorigate/src/change-notes/released/1.0.6.md

@@ -1 +0,0 @@
-## 1.0.6

csharp/ql/campaigns/Solorigate/src/change-notes/released/1.0.7.md

@@ -1 +0,0 @@
-## 1.0.7

csharp/ql/campaigns/Solorigate/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 1.0.7
+lastReleaseVersion: 1.0.5

csharp/ql/campaigns/Solorigate/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/csharp-solorigate-queries
-version: 1.0.7
+version: 1.0.6-dev
 groups:
     - csharp
     - solorigate

csharp/ql/lib/CHANGELOG.md

@@ -1,24 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.
-
 ## 0.0.11
 
 ### Breaking Changes

csharp/ql/lib/change-notes/2022-02-07-deleted-deprecations.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

csharp/ql/lib/change-notes/2022-02-07-deprecated-acronyms.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.

csharp/ql/lib/change-notes/2022-03-14-flow-state-barriers.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.

csharp/ql/lib/change-notes/2022-03-14-taint-interface-cleanup.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.

csharp/ql/lib/change-notes/released/0.0.12.md

@@ -1,18 +0,0 @@
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

csharp/ql/lib/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

csharp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/csharp-all
-version: 0.0.13
+version: 0.0.12-dev
 groups: csharp
 dbscheme: semmlecode.csharp.dbscheme
 extractor: csharp

csharp/ql/lib/semmle/code/csharp/commons/Constants.qll

@@ -105,31 +105,12 @@ private module ConstantComparisonOperation {
   }
 }
 
-private class StructuralComparisonConfig extends StructuralComparison::StructuralComparisonConfiguration {
-  StructuralComparisonConfig() { this = "CompareIdenticalValues" }
-
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(ComparisonTest ct |
-      x = ct.getFirstArgument() and
-      y = ct.getSecondArgument()
-    )
-  }
-
-  ComparisonTest getComparisonTest() {
-    exists(Element x, Element y |
-      result.getFirstArgument() = x and
-      result.getSecondArgument() = y and
-      same(x, y)
-    )
-  }
-}
-
 /**
  * Holds if comparison test `ct` compares two structurally identical
  * expressions.
  */
 predicate comparesIdenticalValues(ComparisonTest ct) {
-  ct = any(StructuralComparisonConfig c).getComparisonTest()
+  StructuralComparison::sameGvn(ct.getFirstArgument(), ct.getSecondArgument())
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/commons/StructuralComparison.qll

@@ -192,13 +192,18 @@ private import Cached
 
 predicate toGvn = toGvnCached/1;
 
+/**
+ * Holds if the control flow elements `x` and `y` are structurally equal.
+ */
 pragma[inline]
-private predicate sameGvn(ControlFlowElement x, ControlFlowElement y) {
+predicate sameGvn(ControlFlowElement x, ControlFlowElement y) {
   pragma[only_bind_into](toGvn(pragma[only_bind_out](x))) =
     pragma[only_bind_into](toGvn(pragma[only_bind_out](y)))
 }
 
 /**
+ * DEPRECATED: Use `sameGvn` instead.
+ *
  * A configuration for performing structural comparisons of program elements
  * (expressions and statements).
  *
@@ -207,7 +212,7 @@ private predicate sameGvn(ControlFlowElement x, ControlFlowElement y) {
  *
  * Each use of the library is identified by a unique string value.
  */
-abstract class StructuralComparisonConfiguration extends string {
+abstract deprecated class StructuralComparisonConfiguration extends string {
   bindingset[this]
   StructuralComparisonConfiguration() { any() }
 
@@ -235,55 +240,3 @@ abstract class StructuralComparisonConfiguration extends string {
    */
   predicate same(ControlFlowElement x, ControlFlowElement y) { candidate(x, y) and sameGvn(x, y) }
 }
-
-/**
- * INTERNAL: Do not use.
- *
- * A verbatim copy of the class `StructuralComparisonConfiguration` for internal
- * use.
- *
- * A copy is needed in order to use structural comparison within the standard
- * library without running into caching issues.
- */
-module Internal {
-  // Import all uses of the internal library to make sure caching works
-  private import semmle.code.csharp.controlflow.Guards as G
-
-  /**
-   * A configuration for performing structural comparisons of program elements
-   * (expressions and statements).
-   *
-   * The predicate `candidate()` must be overridden, in order to identify the
-   * elements for which to perform structural comparison.
-   *
-   * Each use of the library is identified by a unique string value.
-   */
-  abstract class InternalStructuralComparisonConfiguration extends string {
-    bindingset[this]
-    InternalStructuralComparisonConfiguration() { any() }
-
-    /**
-     * Holds if elements `x` and `y` are candidates for testing structural
-     * equality.
-     *
-     * Subclasses are expected to override this predicate to identify the
-     * top-level elements which they want to compare. Care should be
-     * taken to avoid identifying too many pairs of elements, as in general
-     * there are very many structurally equal subtrees in a program, and
-     * in order to keep the computation feasible we must focus attention.
-     *
-     * Note that this relation is not expected to be symmetric -- it's
-     * fine to include a pair `(x, y)` but not `(y, x)`.
-     * In fact, not including the symmetrically implied fact will save
-     * half the computation time on the structural comparison.
-     */
-    abstract predicate candidate(ControlFlowElement x, ControlFlowElement y);
-
-    /**
-     * Holds if elements `x` and `y` structurally equal. `x` and `y` must be
-     * flagged as candidates for structural equality, that is,
-     * `candidate(x, y)` must hold.
-     */
-    predicate same(ControlFlowElement x, ControlFlowElement y) { candidate(x, y) and sameGvn(x, y) }
-  }
-}

csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll

@@ -8,7 +8,7 @@ private import dotnet
 private import ControlFlow::SuccessorTypes
 private import semmle.code.csharp.commons.Assertions
 private import semmle.code.csharp.commons.ComparisonTest
-private import semmle.code.csharp.commons.StructuralComparison::Internal
+private import semmle.code.csharp.commons.StructuralComparison as SC
 private import semmle.code.csharp.controlflow.BasicBlocks
 private import semmle.code.csharp.controlflow.internal.Completion
 private import semmle.code.csharp.frameworks.System
@@ -1798,32 +1798,30 @@ module Internal {
   }
 
   /**
-   * A helper class for calculating structurally equal access/call expressions.
+   * Holds if access/call expression `e` (targeting declaration `target`)
+   * is a sub expression of a guard that controls whether basic block
+   * `bb` is reached.
    */
-  private class ConditionOnExprComparisonConfig extends InternalStructuralComparisonConfiguration {
-    ConditionOnExprComparisonConfig() { this = "ConditionOnExprComparisonConfig" }
+  pragma[noinline]
+  private predicate candidateAux(AccessOrCallExpr e, Declaration target, BasicBlock bb) {
+    target = e.getTarget() and
+    guardControlsSub(_, bb, e)
+  }
 
-    override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-      exists(BasicBlock bb, Declaration d |
-        this.candidateAux(x, d, bb) and
-        y =
-          any(AccessOrCallExpr e |
-            e.getAControlFlowNode().getBasicBlock() = bb and
-            e.getTarget() = d
-          )
-      )
-    }
+  private predicate candidate(AccessOrCallExpr x, AccessOrCallExpr y) {
+    exists(BasicBlock bb, Declaration d |
+      candidateAux(x, d, bb) and
+      y =
+        any(AccessOrCallExpr e |
+          e.getAControlFlowNode().getBasicBlock() = bb and
+          e.getTarget() = d
+        )
+    )
+  }
 
-    /**
-     * Holds if access/call expression `e` (targeting declaration `target`)
-     * is a sub expression of a guard that controls whether basic block
-     * `bb` is reached.
-     */
-    pragma[noinline]
-    private predicate candidateAux(AccessOrCallExpr e, Declaration target, BasicBlock bb) {
-      target = e.getTarget() and
-      guardControlsSub(_, bb, e)
-    }
+  private predicate same(AccessOrCallExpr x, AccessOrCallExpr y) {
+    candidate(x, y) and
+    SC::sameGvn(x, y)
   }
 
   cached
@@ -1849,7 +1847,7 @@ module Internal {
     pragma[nomagic]
     private predicate guardControlsSubSame(Guard g, BasicBlock bb, ControlGuardDescendant sub) {
       guardControlsSub(g, bb, sub) and
-      any(ConditionOnExprComparisonConfig c).same(sub, _)
+      same(sub, _)
     }
 
     pragma[nomagic]
@@ -1862,7 +1860,7 @@ module Internal {
       guardedBB = guardedCfn.getBasicBlock() and
       guardControls(g, guardedBB, v) and
       guardControlsSubSame(g, guardedBB, sub) and
-      any(ConditionOnExprComparisonConfig c).same(sub, guarded)
+      same(sub, guarded)
     }
 
     pragma[nomagic]

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -2031,3 +2031,47 @@ abstract class SyntheticField extends string {
  * Holds if the the content `c` is a container.
  */
 predicate containerContent(DataFlow::Content c) { c instanceof DataFlow::ElementContent }
+
+/**
+ * A module containing predicates related to generating models as data.
+ */
+module Csv {
+  private string parameterQualifiedTypeNamesToString(DataFlowCallable c) {
+    result =
+      concat(Parameter p, int i |
+        p = c.getParameter(i)
+      |
+        p.getType().getQualifiedName(), "," order by i
+      )
+  }
+
+  /** Holds if the summary should apply for all overrides of `c`. */
+  predicate isBaseCallableOrPrototype(DataFlowCallable c) {
+    c.getDeclaringType() instanceof Interface
+    or
+    exists(Modifiable m | m = [c.(Modifiable), c.(Accessor).getDeclaration()] |
+      m.isAbstract()
+      or
+      c.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
+    )
+  }
+
+  /** Gets a string representing whether the summary should apply for all overrides of `c`. */
+  private string getCallableOverride(DataFlowCallable c) {
+    if isBaseCallableOrPrototype(c) then result = "true" else result = "false"
+  }
+
+  /** Computes the first 6 columns for CSV rows of `c`. */
+  string asPartialModel(DataFlowCallable c) {
+    exists(string namespace, string type |
+      c.getDeclaringType().hasQualifiedName(namespace, type) and
+      result =
+        namespace + ";" //
+          + type + ";" //
+          + getCallableOverride(c) + ";" //
+          + c.getName() + ";" //
+          + "(" + parameterQualifiedTypeNamesToString(c) + ")" //
+          + /* ext + */ ";" //
+    )
+  }
+}

csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

@@ -1056,7 +1056,7 @@ module Private {
       |
         c.relevantSummary(input, output, preservesValue) and
         csv =
-          c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
+          c.getCallableCsv() + ";" + getComponentStackCsv(input) + ";" +
             getComponentStackCsv(output) + ";" + renderKind(preservesValue)
       )
     }

csharp/ql/src/CHANGELOG.md

@@ -1,7 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
 ## 0.0.11
 
 ### Minor Analysis Improvements

csharp/ql/src/Concurrency/UnsafeLazyInitialization.ql

@@ -14,24 +14,12 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-class DoubleCheckedLock extends StructuralComparisonConfiguration {
-  DoubleCheckedLock() { this = "double checked lock" }
-
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(IfStmt unlockedIf, IfStmt lockedIf, LockStmt lock |
-      x = unlockedIf.getCondition() and
-      y = lockedIf.getCondition() and
-      lock = unlockedIf.getThen().stripSingletonBlocks() and
-      lockedIf.getParent*() = lock.getBlock()
-    )
-  }
-}
-
-predicate doubleCheckedLock(Field field, IfStmt ifs) {
-  exists(DoubleCheckedLock config, LockStmt lock, Expr eq1, Expr eq2 | ifs.getCondition() = eq1 |
-    lock = ifs.getThen().stripSingletonBlocks() and
-    config.same(eq1, eq2) and
-    field.getAnAccess() = eq1.getAChildExpr*()
+predicate doubleCheckedLock(Field field, IfStmt unlockedIf) {
+  exists(LockStmt lock, IfStmt lockedIf |
+    lock = unlockedIf.getThen().stripSingletonBlocks() and
+    lockedIf.getParent*() = lock.getBlock() and
+    sameGvn(unlockedIf.getCondition(), lockedIf.getCondition()) and
+    field.getAnAccess() = unlockedIf.getCondition().getAChildExpr*()
   )
 }
 

csharp/ql/src/Language Abuse/MissedTernaryOpportunity.ql

@@ -12,25 +12,8 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() { this = "MissedTernaryOpportunity" }
-
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(IfStmt is, AssignExpr ae1 |
-      ae1 = is.getThen().stripSingletonBlocks().(ExprStmt).getExpr()
-    |
-      x = ae1.getLValue() and
-      exists(AssignExpr ae2 | ae2 = is.getElse().stripSingletonBlocks().(ExprStmt).getExpr() |
-        y = ae2.getLValue()
-      )
-    )
-  }
-
-  IfStmt getIfStmt() {
-    exists(AssignExpr ae | ae = result.getThen().stripSingletonBlocks().(ExprStmt).getExpr() |
-      same(ae.getLValue(), _)
-    )
-  }
+private Expr getAssignedExpr(Stmt stmt) {
+  result = stmt.stripSingletonBlocks().(ExprStmt).getExpr().(AssignExpr).getLValue()
 }
 
 from IfStmt is, string what
@@ -40,10 +23,8 @@ where
     is.getElse().stripSingletonBlocks() instanceof ReturnStmt and
     what = "return"
     or
-    exists(StructuralComparisonConfig c |
-      is = c.getIfStmt() and
-      what = "write to the same variable"
-    )
+    sameGvn(getAssignedExpr(is.getThen()), getAssignedExpr(is.getElse())) and
+    what = "write to the same variable"
   ) and
   not exists(IfStmt other | is = other.getElse())
 select is,

csharp/ql/src/Language Abuse/UselessIsBeforeAs.ql

@@ -13,35 +13,26 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() { this = "UselessIsBeforeAs" }
+private predicate candidate(AsExpr ae, IsExpr ie) {
+  exists(IfStmt is, TypeAccessPatternExpr tape |
+    ie = is.getCondition().getAChild*() and
+    tape = ie.getPattern() and
+    ae.getTargetType() = tape.getTarget()
+  |
+    ae = is.getThen().getAChild*()
+    or
+    ae = is.getElse().getAChild*()
+  )
+}
 
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(IfStmt is, AsExpr ae, IsExpr ie, TypeAccessPatternExpr tape |
-      ie = is.getCondition().getAChild*() and
-      tape = ie.getPattern() and
-      ae.getTargetType() = tape.getTarget() and
-      x = ie.getExpr() and
-      y = ae.getExpr()
-    |
-      ae = is.getThen().getAChild*()
-      or
-      ae = is.getElse().getAChild*()
-    )
-  }
-
-  predicate uselessIsBeforeAs(AsExpr ae, IsExpr ie) {
-    exists(Expr x, Expr y |
-      same(x, y) and
-      ie.getExpr() = x and
-      ae.getExpr() = y
-    )
-  }
+private predicate uselessIsBeforeAs(AsExpr ae, IsExpr ie) {
+  candidate(ae, ie) and
+  sameGvn(ie.getExpr(), ae.getExpr())
 }
 
 from AsExpr ae, IsExpr ie
 where
-  exists(StructuralComparisonConfig c | c.uselessIsBeforeAs(ae, ie)) and
+  uselessIsBeforeAs(ae, ie) and
   not exists(MethodCall mc | ae = mc.getAnArgument().getAChildExpr*())
 select ae,
   "This 'as' expression performs a type test - it should be directly compared against null, rendering the 'is' $@ potentially redundant.",

csharp/ql/src/Language Abuse/UselessNullCoalescingExpression.ql

@@ -14,24 +14,22 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() { this = "UselessNullCoalescingExpression" }
-
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(NullCoalescingExpr nce |
-      x.(Access) = nce.getLeftOperand() and
-      y.(Access) = nce.getRightOperand().getAChildExpr*()
-    )
-  }
-
-  NullCoalescingExpr getUselessNullCoalescingExpr() {
-    exists(AssignableAccess x |
-      result.getLeftOperand() = x and
-      forex(AssignableAccess y | same(x, y) | y instanceof AssignableRead and not y.isRefArgument())
-    )
-  }
+pragma[noinline]
+private predicate same(AssignableAccess x, AssignableAccess y) {
+  exists(NullCoalescingExpr nce |
+    x = nce.getLeftOperand() and
+    y = nce.getRightOperand().getAChildExpr*()
+  ) and
+  sameGvn(x, y)
 }
 
-from StructuralComparisonConfig c, NullCoalescingExpr nce
-where nce = c.getUselessNullCoalescingExpr()
+private predicate uselessNullCoalescingExpr(NullCoalescingExpr nce) {
+  exists(AssignableAccess x |
+    nce.getLeftOperand() = x and
+    forex(AssignableAccess y | same(x, y) | y instanceof AssignableRead and not y.isRefArgument())
+  )
+}
+
+from NullCoalescingExpr nce
+where uselessNullCoalescingExpr(nce)
 select nce, "Both operands of this null-coalescing expression access the same variable or property."

csharp/ql/src/Likely Bugs/NestedLoopsSameVariable.ql

@@ -15,18 +15,6 @@ import csharp
 import semmle.code.csharp.commons.ComparisonTest
 import semmle.code.csharp.commons.StructuralComparison as SC
 
-/** A structural comparison configuration for comparing the conditions of nested `for` loops. */
-class NestedForConditions extends SC::StructuralComparisonConfiguration {
-  NestedForConditions() { this = "Compare nested for conditions" }
-
-  override predicate candidate(ControlFlowElement e1, ControlFlowElement e2) {
-    exists(NestedForLoopSameVariable nested |
-      e1 = nested.getInnerForStmt().getCondition() and
-      e2 = nested.getOuterForStmt().getCondition()
-    )
-  }
-}
-
 private predicate hasChild(Stmt outer, Element child) {
   outer = child.getParent() and
   (outer instanceof ForStmt or outer = any(ForStmt f).getBody())
@@ -61,9 +49,7 @@ class NestedForLoopSameVariable extends ForStmt {
   }
 
   private predicate haveSameCondition() {
-    exists(NestedForConditions config |
-      config.same(this.getInnerForStmt().getCondition(), this.getOuterForStmt().getCondition())
-    )
+    SC::sameGvn(this.getInnerForStmt().getCondition(), this.getOuterForStmt().getCondition())
   }
 
   private predicate haveSameUpdate() {

csharp/ql/src/Likely Bugs/SelfAssignment.ql

@@ -13,37 +13,25 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-class StructuralComparisonConfig extends StructuralComparisonConfiguration {
-  StructuralComparisonConfig() { this = "SelfAssignment" }
-
-  override predicate candidate(ControlFlowElement x, ControlFlowElement y) {
-    exists(AssignExpr ae |
-      // Member initializers are never self-assignments, in particular
-      // not initializers such as `new C { F = F };`
-      not ae instanceof MemberInitializer and
-      // Enum field initializers are never self assignments. `enum E { A = 42 }`
-      not ae.getParent().(Field).getDeclaringType() instanceof Enum
-    |
-      ae.getLValue() = x and
-      ae.getRValue() = y
-    ) and
-    forall(Expr e | e = x.(Expr).getAChildExpr*() |
-      // Non-trivial property accesses may have side-effects,
-      // so these are not considered
-      e instanceof PropertyAccess implies e instanceof TrivialPropertyAccess
-    )
-  }
-
-  AssignExpr getSelfAssignExpr() {
-    exists(Expr x, Expr y |
-      same(x, y) and
-      result.getLValue() = x and
-      result.getRValue() = y
-    )
-  }
+private predicate candidate(AssignExpr ae) {
+  // Member initializers are never self-assignments, in particular
+  // not initializers such as `new C { F = F };`
+  not ae instanceof MemberInitializer and
+  // Enum field initializers are never self assignments. `enum E { A = 42 }`
+  not ae.getParent().(Field).getDeclaringType() instanceof Enum and
+  forall(Expr e | e = ae.getLValue().getAChildExpr*() |
+    // Non-trivial property accesses may have side-effects,
+    // so these are not considered
+    e instanceof PropertyAccess implies e instanceof TrivialPropertyAccess
+  )
 }
 
-Declaration getDeclaration(Expr e) {
+private predicate selfAssignExpr(AssignExpr ae) {
+  candidate(ae) and
+  sameGvn(ae.getLValue(), ae.getRValue())
+}
+
+private Declaration getDeclaration(Expr e) {
   result = e.(VariableAccess).getTarget()
   or
   result = e.(MemberAccess).getTarget()
@@ -51,6 +39,6 @@ Declaration getDeclaration(Expr e) {
   result = getDeclaration(e.(ArrayAccess).getQualifier())
 }
 
-from StructuralComparisonConfig c, AssignExpr ae, Declaration target
-where ae = c.getSelfAssignExpr() and target = getDeclaration(ae.getLValue())
+from AssignExpr ae, Declaration target
+where selfAssignExpr(ae) and target = getDeclaration(ae.getLValue())
 select ae, "This assignment assigns $@ to itself.", target, target.getName()

csharp/ql/src/Performance/UseTryGetValue.ql

@@ -13,19 +13,14 @@ import csharp
 import semmle.code.csharp.commons.StructuralComparison
 import semmle.code.csharp.controlflow.Guards as G
 
-class SameElement extends StructuralComparisonConfiguration {
-  SameElement() { this = "Same element" }
-
-  override predicate candidate(ControlFlowElement e1, ControlFlowElement e2) {
-    exists(MethodCall mc, IndexerRead access |
-      mc.getTarget().hasName("ContainsKey") and
-      access.getQualifier().(G::GuardedExpr).isGuardedBy(mc, mc.getQualifier(), _) and
-      e1 = mc.getArgument(0) and
-      e2 = access.getIndex(0)
-    )
-  }
+pragma[noinline]
+private predicate candidate(MethodCall mc, IndexerRead access) {
+  mc.getTarget().hasName("ContainsKey") and
+  access.getQualifier().(G::GuardedExpr).isGuardedBy(mc, mc.getQualifier(), _)
 }
 
-from SameElement element, MethodCall call, IndexerAccess index
-where element.same(call.getArgument(0), index.getIndex(0))
+from MethodCall call, IndexerRead index
+where
+  candidate(call, index) and
+  sameGvn(call.getArgument(0), index.getIndex(0))
 select call, "Inefficient use of 'ContainsKey' and $@.", index, "indexer"

csharp/ql/src/change-notes/released/0.0.12.md

@@ -1 +0,0 @@
-## 0.0.12

csharp/ql/src/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

csharp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/csharp-queries
-version: 0.0.13
+version: 0.0.12-dev
 groups: 
   - csharp
   - queries

csharp/ql/src/utils/model-generator/ModelGeneratorUtilsSpecific.qll

@@ -7,12 +7,12 @@ private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
 private predicate isRelevantForModels(Callable api) { not api instanceof MainMethod }
 
 /**
- * A class of Callables that are relevant for generating summary, source and sinks models for.
+ * A class of callables that are relevant generating summary, source and sinks models for.
  *
- * In the Standard library and 3rd party libraries it the Callables that can be called
+ * In the Standard library and 3rd party libraries it the callables that can be called
  * from outside the library itself.
  */
-class TargetApi extends Callable {
+class TargetApi extends DataFlowCallable {
   TargetApi() {
     [this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic() and
     this.fromSource() and
@@ -20,44 +20,7 @@ class TargetApi extends Callable {
   }
 }
 
-private string parameterQualifiedTypeNamesToString(TargetApi api) {
-  result =
-    concat(Parameter p, int i |
-      p = api.getParameter(i)
-    |
-      p.getType().getQualifiedName(), "," order by i
-    )
-}
-
-/** Holds if the summary should apply for all overrides of this. */
-private predicate isBaseCallableOrPrototype(TargetApi api) {
-  api.getDeclaringType() instanceof Interface
-  or
-  exists(Modifiable m | m = [api.(Modifiable), api.(Accessor).getDeclaration()] |
-    m.isAbstract()
-    or
-    api.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
-  )
-}
-
-/** Gets a string representing whether the summary should apply for all overrides of this. */
-private string getCallableOverride(TargetApi api) {
-  if isBaseCallableOrPrototype(api) then result = "true" else result = "false"
-}
-
-/** Computes the first 6 columns for CSV rows. */
-string asPartialModel(TargetApi api) {
-  exists(string namespace, string type |
-    api.getDeclaringType().hasQualifiedName(namespace, type) and
-    result =
-      namespace + ";" //
-        + type + ";" //
-        + getCallableOverride(api) + ";" //
-        + api.getName() + ";" //
-        + "(" + parameterQualifiedTypeNamesToString(api) + ")" //
-        + /* ext + */ ";" //
-  )
-}
+predicate asPartialModel = Csv::asPartialModel/1;
 
 /**
  * Holds for type `t` for fields that are relevant as an intermediate

csharp/ql/test/library-tests/dataflow/library/FlowSummariesFiltered.ql

@@ -1,4 +1,5 @@
 import shared.FlowSummaries
+private import semmle.code.csharp.dataflow.internal.DataFlowPrivate::Csv
 private import semmle.code.csharp.dataflow.ExternalFlow
 
 class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
@@ -14,7 +15,7 @@ class IncludeFilteredSummarizedCallable extends IncludeSummarizedCallable {
   ) {
     this.propagatesFlow(input, output, preservesValue) and
     not exists(IncludeSummarizedCallable rsc |
-      rsc.isBaseCallableOrPrototype() and
+      isBaseCallableOrPrototype(rsc) and
       rsc.propagatesFlow(input, output, preservesValue) and
       this.(UnboundCallable).overridesOrImplementsUnbound(rsc)
     )

csharp/ql/test/library-tests/structuralcomparison/structuralComparison.ql

@@ -1,28 +1,25 @@
 import csharp
 import semmle.code.csharp.commons.StructuralComparison
 
-private class StructuralComparisonTest extends StructuralComparisonConfiguration {
-  StructuralComparisonTest() { this = "StructuralComparisonTest" }
-
-  /**
-   * All pairs of controls flow elements found in the source and within the same
-   * enclosing callable excluding all instances of `ThisAccess` to reduce the size
-   * of the output.
-   */
-  override predicate candidate(ControlFlowElement e1, ControlFlowElement e2) {
-    e1.fromSource() and
-    e2.fromSource() and
-    e1 != e2 and
-    e1.getEnclosingCallable() = e2.getEnclosingCallable() and
-    not e1 instanceof ThisAccess
-  }
+/**
+ * All pairs of controls flow elements found in the source and within the same
+ * enclosing callable excluding all instances of `ThisAccess` to reduce the size
+ * of the output.
+ */
+private predicate candidate(ControlFlowElement x, ControlFlowElement y) {
+  x.fromSource() and
+  y.fromSource() and
+  x != y and
+  x.getEnclosingCallable() = y.getEnclosingCallable() and
+  not x instanceof ThisAccess
 }
 
-query predicate same(ControlFlowElement e1, ControlFlowElement e2) {
-  exists(StructuralComparisonTest sct, Location l1, Location l2 |
-    sct.same(e1, e2) and
-    l1 = e1.getLocation() and
-    l2 = e2.getLocation() and
+query predicate same(ControlFlowElement x, ControlFlowElement y) {
+  exists(Location l1, Location l2 |
+    candidate(x, y) and
+    sameGvn(x, y) and
+    l1 = x.getLocation() and
+    l2 = y.getLocation() and
     (
       l1.getStartLine() < l2.getStartLine()
       or
@@ -31,4 +28,4 @@ query predicate same(ControlFlowElement e1, ControlFlowElement e2) {
   )
 }
 
-query predicate gvn(ControlFlowElement e, Gvn gvn) { gvn = toGvn(e) and e.fromSource() }
+query predicate gvn(ControlFlowElement x, Gvn gvn) { gvn = toGvn(x) and x.fromSource() }

csharp/ql/test/shared/FlowSummaries.qll

@@ -1,44 +1,12 @@
 import semmle.code.csharp.dataflow.FlowSummary
 import semmle.code.csharp.dataflow.internal.FlowSummaryImpl::Private::TestOutput
+private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
 
 abstract class IncludeSummarizedCallable extends RelevantSummarizedCallable {
   IncludeSummarizedCallable() {
     [this.(Modifiable), this.(Accessor).getDeclaration()].isEffectivelyPublic()
   }
 
-  /** Gets the qualified parameter types of this callable as a comma-separated string. */
-  private string parameterQualifiedTypeNamesToString() {
-    result =
-      concat(Parameter p, int i |
-        p = this.getParameter(i)
-      |
-        p.getType().getQualifiedName(), "," order by i
-      )
-  }
-
-  /** Holds if the summary should apply for all overrides of this. */
-  predicate isBaseCallableOrPrototype() {
-    this.getDeclaringType() instanceof Interface
-    or
-    exists(Modifiable m | m = [this.(Modifiable), this.(Accessor).getDeclaration()] |
-      m.isAbstract()
-      or
-      this.getDeclaringType().(Modifiable).isAbstract() and m.(Virtualizable).isVirtual()
-    )
-  }
-
-  /** Gets a string representing whether the summary should apply for all overrides of this. */
-  private string getCallableOverride() {
-    if this.isBaseCallableOrPrototype() then result = "true" else result = "false"
-  }
-
   /** Gets a string representing the callable in semi-colon separated format for use in flow summaries. */
-  final override string getCallableCsv() {
-    exists(string namespace, string type |
-      this.getDeclaringType().hasQualifiedName(namespace, type) and
-      result =
-        namespace + ";" + type + ";" + this.getCallableOverride() + ";" + this.getName() + ";" + "("
-          + this.parameterQualifiedTypeNamesToString() + ")"
-    )
-  }
+  final override string getCallableCsv() { result = Csv::asPartialModel(this) }
 }

java/ql/lib/CHANGELOG.md

@@ -1,28 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
- * Added new guards `IsWindowsGuard`, `IsSpecificWindowsVariant`, `IsUnixGuard`, and `IsSpecificUnixVariant` to detect OS specific guards.
- * Added a new predicate `getSystemProperty` that gets all expressions that retrieve system properties from a variety of sources (eg. alternative JDK API's, Google Guava, Apache Commons, Apache IO, etc.).
- * Added support for detection of SSRF via JDBC database URLs, including connections made using the standard library (`java.sql`), Hikari Connection Pool, JDBI and Spring JDBC.
- * Re-removed support for `CharacterLiteral` from `CompileTimeConstantExpr.getStringValue()` to restore the convention that that predicate only applies to `String`-typed constants.
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.
-
 ## 0.0.11
 
 ### New Features

java/ql/lib/change-notes/2022-02-07-deleted-deprecations.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

java/ql/lib/change-notes/2022-02-07-deprecated-acronyms.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.

java/ql/lib/change-notes/2022-03-11-revert-8325.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+ * Re-removed support for `CharacterLiteral` from `CompileTimeConstantExpr.getStringValue()` to restore the convention that that predicate only applies to `String`-typed constants.

java/ql/lib/change-notes/2022-03-14-flow-state-barriers.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.

java/ql/lib/change-notes/2022-03-14-new-jdbc-ssrf-sinks.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+ * Added support for detection of SSRF via JDBC database URLs, including connections made using the standard library (`java.sql`), Hikari Connection Pool, JDBI and Spring JDBC.

java/ql/lib/change-notes/2022-03-14-taint-interface-cleanup.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.

java/ql/lib/change-notes/released/0.0.12.md

@@ -1,22 +0,0 @@
-## 0.0.12
-
-### Breaking Changes
-
-* The flow state variants of `isBarrier` and `isAdditionalFlowStep` are no longer exposed in the taint tracking library. The `isSanitizer` and `isAdditionalTaintStep` predicates should be used instead.
-
-### Deprecated APIs
-
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### New Features
-
-* The data flow and taint tracking libraries have been extended with versions of `isBarrierIn`, `isBarrierOut`, and `isBarrierGuard`, respectively `isSanitizerIn`, `isSanitizerOut`, and `isSanitizerGuard`, that support flow states.
-
-### Minor Analysis Improvements
-
- * Added new guards `IsWindowsGuard`, `IsSpecificWindowsVariant`, `IsUnixGuard`, and `IsSpecificUnixVariant` to detect OS specific guards.
- * Added a new predicate `getSystemProperty` that gets all expressions that retrieve system properties from a variety of sources (eg. alternative JDK API's, Google Guava, Apache Commons, Apache IO, etc.).
- * Added support for detection of SSRF via JDBC database URLs, including connections made using the standard library (`java.sql`), Hikari Connection Pool, JDBI and Spring JDBC.
- * Re-removed support for `CharacterLiteral` from `CompileTimeConstantExpr.getStringValue()` to restore the convention that that predicate only applies to `String`-typed constants.
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

java/ql/lib/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

java/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/java-all
-version: 0.0.13
+version: 0.0.12-dev
 groups: java
 dbscheme: config/semmlecode.dbscheme
 extractor: java

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll

@@ -1056,7 +1056,7 @@ module Private {
       |
         c.relevantSummary(input, output, preservesValue) and
         csv =
-          c.getCallableCsv() + ";;" + getComponentStackCsv(input) + ";" +
+          c.getCallableCsv() + ";" + getComponentStackCsv(input) + ";" +
             getComponentStackCsv(output) + ";" + renderKind(preservesValue)
       )
     }

java/ql/src/CHANGELOG.md

@@ -1,15 +1,3 @@
-## 0.0.13
-
-## 0.0.12
-
-### New Queries
-
-* The query "Insertion of sensitive information into log files" (`java/sensitive-logging`) has been promoted from experimental to the main query pack. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3090).
-
-### Minor Analysis Improvements
-
- * Updated "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) to remove false-positives when OS is properly used as logical guard.
-
 ## 0.0.11
 
 ## 0.0.10

java/ql/src/change-notes/2022-02-14-os-guards.md

@@ -0,0 +1,7 @@
+---
+category: minorAnalysis
+---
+ * Added new guards `IsWindowsGuard`, `IsSpecificWindowsVariant`, `IsUnixGuard`, and `IsSpecificUnixVariant` to detect OS specific guards.
+ * Added a new predicate `getSystemProperty` that gets all expressions that retrieve system properties from a variety of sources (eg. alternative JDK API's, Google Guava, Apache Commons, Apache IO, etc..).
+ * Updated "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) to remove false-positives when OS is properly used as logical guard.
+ 

java/ql/src/change-notes/2022-03-11-sensitive-logging.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* The query "Insertion of sensitive information into log files" (`java/sensitive-logging`) has been promoted from experimental to the main query pack. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3090).

java/ql/src/change-notes/released/0.0.12.md

@@ -1,9 +0,0 @@
-## 0.0.12
-
-### New Queries
-
-* The query "Insertion of sensitive information into log files" (`java/sensitive-logging`) has been promoted from experimental to the main query pack. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/3090).
-
-### Minor Analysis Improvements
-
- * Updated "Local information disclosure in a temporary directory" (`java/local-temp-file-or-directory-information-disclosure`) to remove false-positives when OS is properly used as logical guard.

java/ql/src/change-notes/released/0.0.13.md

@@ -1 +0,0 @@
-## 0.0.13

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.13
+lastReleaseVersion: 0.0.11

java/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/java-queries
-version: 0.0.13
+version: 0.0.12-dev
 groups: 
   - java
   - queries

java/ql/src/utils/model-generator/ModelGeneratorUtilsSpecific.qll

@@ -5,7 +5,7 @@ private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.internal.ContainerFlow
 private import semmle.code.java.dataflow.internal.DataFlowImplCommon
 
-Method superImpl(Method m) {
+private Method superImpl(Method m) {
   result = m.getAnOverride() and
   not exists(result.getAnOverride()) and
   not m instanceof ToStringMethod

javascript/ql/lib/CHANGELOG.md

@@ -1,20 +1,3 @@
-## 0.0.14
-
-## 0.0.13
-
-### Deprecated APIs
-
-* Some predicates from `DefUse.qll`, `DataFlow.qll`, `TaintTracking.qll`, `DOM.qll`, `Definitions.qll` that weren't used by any query have been deprecated. 
-  The documentation for each predicate points to an alternative.
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-* Some modules that started with a lowercase letter have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### Minor Analysis Improvements
-
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.
-
 ## 0.0.12
 
 ### Major Analysis Improvements

javascript/ql/lib/change-notes/2022-02-07-deleted-deprecations.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

javascript/ql/lib/change-notes/2022-02-07-deprecated-acronyms.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.

javascript/ql/lib/change-notes/2022-02-07-deprecated-modules.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Some modules that started with a lowercase letter have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.

javascript/ql/lib/change-notes/2022-02-14-deprecated-predicates.md

@@ -0,0 +1,5 @@
+---
+category: deprecated
+---
+* Some predicates from `DefUse.qll`, `DataFlow.qll`, `TaintTracking.qll`, `DOM.qll`, `Definitions.qll` that weren't used by any query have been deprecated. 
+  The documentation for each predicate points to an alternative.

javascript/ql/lib/change-notes/released/0.0.13.md

@@ -1,14 +0,0 @@
-## 0.0.13
-
-### Deprecated APIs
-
-* Some predicates from `DefUse.qll`, `DataFlow.qll`, `TaintTracking.qll`, `DOM.qll`, `Definitions.qll` that weren't used by any query have been deprecated. 
-  The documentation for each predicate points to an alternative.
-* Many classes/predicates/modules that had upper-case acronyms have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-* Some modules that started with a lowercase letter have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.
-
-### Minor Analysis Improvements
-
-* All deprecated predicates/classes/modules that have been deprecated for over a year have been deleted.

javascript/ql/lib/change-notes/released/0.0.14.md

@@ -1 +0,0 @@
-## 0.0.14

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.14
+lastReleaseVersion: 0.0.12

javascript/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/javascript-all
-version: 0.0.14
+version: 0.0.13-dev
 groups: javascript
 dbscheme: semmlecode.javascript.dbscheme
 extractor: javascript

javascript/ql/lib/semmle/javascript/DOM.qll

@@ -354,35 +354,6 @@ module DOM {
           call.getNumArgument() = 1 and
           unique(InferredType t | t = getArgumentTypeFromJQueryMethodGet(call)) = TTNumber()
         )
-        or
-        // A `this` node from a callback given to a `$().each(callback)` call.
-        // purposely not using JQuery::MethodCall to avoid `jquery.each()`.
-        exists(DataFlow::CallNode eachCall | eachCall = JQuery::objectRef().getAMethodCall("each") |
-          this = DataFlow::thisNode(eachCall.getCallback(0).getFunction()) or
-          this = eachCall.getABoundCallbackParameter(0, 1)
-        )
-        or
-        // A read of an array-element from a JQuery object. E.g. `$("#foo")[0]`
-        exists(DataFlow::PropRead read |
-          read = this and read = JQuery::objectRef().getAPropertyRead()
-        |
-          unique(InferredType t | t = read.getPropertyNameExpr().analyze().getAType()) = TTNumber()
-        )
-        or
-        // A receiver node of an event handler on a DOM node
-        exists(DataFlow::SourceNode domNode, DataFlow::FunctionNode eventHandler |
-          // NOTE: we do not use `getABoundFunctionValue()`, since bound functions tend to have
-          // a different receiver anyway
-          eventHandler = domNode.getAPropertySource(any(string n | n.matches("on%")))
-          or
-          eventHandler =
-            domNode.getAMethodCall("addEventListener").getArgument(1).getAFunctionValue()
-        |
-          domNode = domValueRef() and
-          this = eventHandler.getReceiver()
-        )
-        or
-        this = DataFlow::thisNode(any(EventHandlerCode evt))
       }
     }
   }
@@ -416,11 +387,6 @@ module DOM {
     or
     t.start() and
     result = domValueRef().getAMethodCall(["item", "namedItem"])
-    or
-    t.startInProp("target") and
-    result = domEventSource()
-    or
-    exists(DataFlow::TypeTracker t2 | result = domValueRef(t2).track(t2, t))
   }
 
   /** Gets a data flow node that may refer to a value from the DOM. */

javascript/ql/lib/semmle/javascript/Promises.qll

@@ -183,12 +183,12 @@ module Promises {
   /**
    * Gets the pseudo-field used to describe resolved values in a promise.
    */
-  string valueProp() { result = "$PromiseResolveField$" }
+  string valueProp() { none() }
 
   /**
    * Gets the pseudo-field used to describe rejected values in a promise.
    */
-  string errorProp() { result = "$PromiseRejectField$" }
+  string errorProp() { none() }
 }
 
 /**

javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll

@@ -756,10 +756,10 @@ private class AdditionalFlowStepAsSharedStep extends SharedFlowStep {
  */
 module PseudoProperties {
   bindingset[s]
-  private string pseudoProperty(string s) { result = "$" + s + "$" }
+  private string pseudoProperty(string s) { none() }
 
   bindingset[s, v]
-  private string pseudoProperty(string s, string v) { result = "$" + s + "|" + v + "$" }
+  private string pseudoProperty(string s, string v) { none() }
 
   /**
    * Gets a pseudo-property for the location of elements in a `Set`

javascript/ql/lib/semmle/javascript/frameworks/Angular2.qll

@@ -136,7 +136,7 @@ module Angular2 {
 
   /** Gets a reference to a `DomSanitizer` object. */
   DataFlow::SourceNode domSanitizer() {
-    result.hasUnderlyingType(["@angular/platform-browser", "@angular/core"], "DomSanitizer")
+    result.hasUnderlyingType("@angular/platform-browser", "DomSanitizer")
   }
 
   /** A value that is about to be promoted to a trusted HTML or CSS value. */

javascript/ql/lib/semmle/javascript/frameworks/Express.qll

@@ -927,28 +927,6 @@ module Express {
     override string getCredentialsKind() { result = kind }
   }
 
-  /** A call to `response.sendFile`, considered as a file system access. */
-  private class ResponseSendFileAsFileSystemAccess extends FileSystemReadAccess,
-    DataFlow::MethodCallNode {
-    ResponseSendFileAsFileSystemAccess() {
-      exists(string name | name = "sendFile" or name = "sendfile" |
-        this.calls(any(ResponseExpr res).flow(), name)
-      )
-    }
-
-    override DataFlow::Node getADataNode() { none() }
-
-    override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-    override DataFlow::Node getRootPathArgument() {
-      result = this.(DataFlow::CallNode).getOptionArgument(1, "root")
-    }
-
-    override predicate isUpwardNavigationRejected(DataFlow::Node argument) {
-      argument = this.getAPathArgument()
-    }
-  }
-
   /**
    * A function that flows to a route setup.
    */

javascript/ql/lib/semmle/javascript/frameworks/Files.qll

@@ -4,23 +4,6 @@
 
 import javascript
 
-/**
- * A call that can produce a file name.
- */
-abstract private class FileNameProducer extends DataFlow::Node {
-  /**
-   * Gets a file name produced by this producer.
-   */
-  abstract DataFlow::Node getAFileName();
-}
-
-/**
- * A node that contains a file name, and is produced by a `ProducesFileNames`.
- */
-private class ProducedFileName extends FileNameSource {
-  ProducedFileName() { this = any(FileNameProducer producer).getAFileName() }
-}
-
 /**
  * A file name from the `walk-sync` library.
  */
@@ -118,319 +101,3 @@ private API::Node fastGlobFileName() {
 private class FastGlobFileNameSource extends FileNameSource {
   FastGlobFileNameSource() { this = fastGlobFileName().getAnImmediateUse() }
 }
-
-/**
- * Classes and predicates for modeling the `fstream` library (https://www.npmjs.com/package/fstream).
- */
-private module FStream {
-  /**
-   * Gets a reference to a method in the `fstream` library.
-   */
-  private DataFlow::SourceNode getAnFStreamProperty(boolean writer) {
-    exists(DataFlow::SourceNode mod, string readOrWrite, string subMod |
-      mod = DataFlow::moduleImport("fstream") and
-      (
-        readOrWrite = "Reader" and writer = false
-        or
-        readOrWrite = "Writer" and writer = true
-      ) and
-      subMod = ["File", "Dir", "Link", "Proxy"]
-    |
-      result = mod.getAPropertyRead(readOrWrite) or
-      result = mod.getAPropertyRead(readOrWrite).getAPropertyRead(subMod) or
-      result = mod.getAPropertyRead(subMod).getAPropertyRead(readOrWrite)
-    )
-  }
-
-  /**
-   * An invocation of a method defined in the `fstream` library.
-   */
-  private class FStream extends FileSystemAccess, DataFlow::InvokeNode {
-    boolean writer;
-
-    FStream() { this = getAnFStreamProperty(writer).getAnInvocation() }
-
-    override DataFlow::Node getAPathArgument() {
-      result = this.getOptionArgument(0, "path")
-      or
-      not exists(this.getOptionArgument(0, "path")) and
-      result = this.getArgument(0)
-    }
-  }
-
-  /**
-   * An invocation of an `fstream` method that writes to a file.
-   */
-  private class FStreamWriter extends FileSystemWriteAccess, FStream {
-    FStreamWriter() { writer = true }
-
-    override DataFlow::Node getADataNode() { none() }
-  }
-
-  /**
-   * An invocation of an `fstream` method that reads a file.
-   */
-  private class FStreamReader extends FileSystemReadAccess, FStream {
-    FStreamReader() { writer = false }
-
-    override DataFlow::Node getADataNode() { none() }
-  }
-}
-
-/**
- * A call to the library `write-file-atomic`.
- */
-private class WriteFileAtomic extends FileSystemWriteAccess, DataFlow::CallNode {
-  WriteFileAtomic() {
-    this = DataFlow::moduleImport("write-file-atomic").getACall()
-    or
-    this = DataFlow::moduleMember("write-file-atomic", "sync").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getADataNode() { result = this.getArgument(1) }
-}
-
-/**
- * A call to the library `recursive-readdir`.
- */
-private class RecursiveReadDir extends FileSystemAccess, FileNameProducer, API::CallNode {
-  RecursiveReadDir() { this = API::moduleImport("recursive-readdir").getACall() }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getAFileName() { result = this.trackFileSource().getAnImmediateUse() }
-
-  private API::Node trackFileSource() {
-    result = this.getParameter([1 .. 2]).getParameter(1)
-    or
-    not exists(this.getCallback([1 .. 2])) and result = this.getReturn().getPromised()
-  }
-}
-
-/**
- * Classes and predicates for modeling the `jsonfile` library (https://www.npmjs.com/package/jsonfile).
- */
-private module JsonFile {
-  /**
-   * A reader for JSON files.
-   */
-  class JsonFileReader extends FileSystemReadAccess, API::CallNode {
-    JsonFileReader() {
-      this = API::moduleImport("jsonfile").getMember(["readFile", "readFileSync"]).getACall()
-    }
-
-    override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-    override DataFlow::Node getADataNode() { result = this.trackRead().getAnImmediateUse() }
-
-    private API::Node trackRead() {
-      this.getCalleeName() = "readFile" and
-      (
-        result = this.getParameter([1 .. 2]).getParameter(1)
-        or
-        not exists(this.getCallback([1 .. 2])) and result = this.getReturn().getPromised()
-      )
-      or
-      this.getCalleeName() = "readFileSync" and
-      result = this.getReturn()
-    }
-  }
-
-  /** DEPRECATED: Alias for JsonFileReader */
-  deprecated class JSONFileReader = JsonFileReader;
-
-  /**
-   * A writer for JSON files.
-   */
-  class JsonFileWriter extends FileSystemWriteAccess, DataFlow::CallNode {
-    JsonFileWriter() {
-      this =
-        DataFlow::moduleMember("jsonfile", any(string s | s = "writeFile" or s = "writeFileSync"))
-            .getACall()
-    }
-
-    override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-    override DataFlow::Node getADataNode() { result = this.getArgument(1) }
-  }
-
-  /** DEPRECATED: Alias for JsonFileWriter */
-  deprecated class JSONFileWriter = JsonFileWriter;
-}
-
-/**
- * A call to the library `load-json-file`.
- */
-private class LoadJsonFile extends FileSystemReadAccess, API::CallNode {
-  LoadJsonFile() {
-    this = API::moduleImport("load-json-file").getACall()
-    or
-    this = API::moduleImport("load-json-file").getMember("sync").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getADataNode() { result = this.trackRead().getAnImmediateUse() }
-
-  private API::Node trackRead() {
-    this.getCalleeName() = "sync" and result = this.getReturn()
-    or
-    not this.getCalleeName() = "sync" and result = this.getReturn().getPromised()
-  }
-}
-
-/**
- * A call to the library `write-json-file`.
- */
-private class WriteJsonFile extends FileSystemWriteAccess, DataFlow::CallNode {
-  WriteJsonFile() {
-    this = DataFlow::moduleImport("write-json-file").getACall()
-    or
-    this = DataFlow::moduleMember("write-json-file", "sync").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getADataNode() { result = this.getArgument(1) }
-}
-
-/**
- * A call to the library `walkdir`.
- */
-private class WalkDir extends FileNameProducer, FileSystemAccess, API::CallNode {
-  WalkDir() {
-    this = API::moduleImport("walkdir").getACall()
-    or
-    this = API::moduleImport("walkdir").getMember("sync").getACall()
-    or
-    this = API::moduleImport("walkdir").getMember("async").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getAFileName() { result = this.trackFileSource().getAnImmediateUse() }
-
-  private API::Node trackFileSource() {
-    not this.getCalleeName() = ["sync", "async"] and
-    (
-      result = this.getParameter(this.getNumArgument() - 1).getParameter(0)
-      or
-      result = this.getReturn().getMember(EventEmitter::on()).getParameter(1).getParameter(0)
-    )
-    or
-    this.getCalleeName() = "sync" and result = this.getReturn()
-    or
-    this.getCalleeName() = "async" and result = this.getReturn().getPromised()
-  }
-}
-
-/**
- * A call to the library `globule`.
- */
-private class Globule extends FileNameProducer, FileSystemAccess, DataFlow::CallNode {
-  Globule() {
-    this = DataFlow::moduleMember("globule", "find").getACall()
-    or
-    this = DataFlow::moduleMember("globule", "match").getACall()
-    or
-    this = DataFlow::moduleMember("globule", "isMatch").getACall()
-    or
-    this = DataFlow::moduleMember("globule", "mapping").getACall()
-    or
-    this = DataFlow::moduleMember("globule", "findMapping").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() {
-    (this.getCalleeName() = "match" or this.getCalleeName() = "isMatch") and
-    result = this.getArgument(1)
-    or
-    this.getCalleeName() = "mapping" and
-    (
-      result = this.getAnArgument() and
-      not exists(result.getALocalSource().getAPropertyWrite("src"))
-      or
-      result = this.getAnArgument().getALocalSource().getAPropertyWrite("src").getRhs()
-    )
-  }
-
-  override DataFlow::Node getAFileName() {
-    result = this and
-    (
-      this.getCalleeName() = "find" or
-      this.getCalleeName() = "match" or
-      this.getCalleeName() = "findMapping" or
-      this.getCalleeName() = "mapping"
-    )
-  }
-}
-
-/**
- * A file system access made by a NodeJS library.
- * This class models multiple NodeJS libraries that access files.
- */
-private class LibraryAccess extends FileSystemAccess, DataFlow::InvokeNode {
-  int pathArgument; // The index of the path argument.
-
-  LibraryAccess() {
-    pathArgument = 0 and
-    (
-      this = DataFlow::moduleImport("path-exists").getACall()
-      or
-      this = DataFlow::moduleImport("rimraf").getACall()
-      or
-      this = DataFlow::moduleImport("readdirp").getACall()
-      or
-      this = DataFlow::moduleImport("walker").getACall()
-      or
-      this =
-        DataFlow::moduleMember("node-dir",
-          ["readFiles", "readFilesStream", "files", "promiseFiles", "subdirs", "paths"]).getACall()
-    )
-    or
-    pathArgument = 0 and
-    this =
-      DataFlow::moduleMember("vinyl-fs", any(string s | s = "src" or s = "dest" or s = "symlink"))
-          .getACall()
-    or
-    pathArgument = [0 .. 1] and
-    (
-      this = DataFlow::moduleImport("ncp").getACall() or
-      this = DataFlow::moduleMember("ncp", "ncp").getACall()
-    )
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(pathArgument) }
-}
-
-/**
- * A call to the library [`chokidar`](https://www.npmjs.com/package/chokidar), where a call to `on` receives file names.
- */
-class Chokidar extends FileNameProducer, FileSystemAccess, API::CallNode {
-  Chokidar() { this = API::moduleImport("chokidar").getMember("watch").getACall() }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-
-  override DataFlow::Node getAFileName() {
-    exists(DataFlow::CallNode onCall, int pathIndex |
-      onCall = this.getAChainedMethodCall("on") and
-      if onCall.getArgument(0).mayHaveStringValue("all") then pathIndex = 1 else pathIndex = 0
-    |
-      result = onCall.getCallback(1).getParameter(pathIndex)
-    )
-  }
-}
-
-/**
- * A call to the [`mkdirp`](https://www.npmjs.com/package/mkdirp) library.
- */
-private class Mkdirp extends FileSystemAccess, API::CallNode {
-  Mkdirp() {
-    this = API::moduleImport("mkdirp").getACall()
-    or
-    this = API::moduleImport("mkdirp").getMember("sync").getACall()
-  }
-
-  override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
-}

javascript/ql/lib/semmle/javascript/frameworks/NoSQL.qll

@@ -8,20 +8,7 @@ import semmle.javascript.Promises
 /** Provices classes for modelling NoSQL query sinks. */
 module NoSql {
   /** An expression that is interpreted as a NoSQL query. */
-  abstract class Query extends Expr {
-    /** Gets an expression that is interpreted as a code operator in this query. */
-    DataFlow::Node getACodeOperator() { none() }
-  }
-}
-
-/** DEPRECATED: Alias for NoSql */
-deprecated module NoSQL = NoSql;
-
-/**
- * Gets a value that has been assigned to the "$where" property of an object that flows to `queryArg`.
- */
-private DataFlow::Node getADollarWhereProperty(API::Node queryArg) {
-  result = queryArg.getMember("$where").getARhs()
+  abstract class Query extends Expr { }
 }
 
 /**
@@ -29,123 +16,112 @@ private DataFlow::Node getADollarWhereProperty(API::Node queryArg) {
  */
 private module MongoDB {
   /**
-   * Gets an access to `mongodb.MongoClient` or a database.
-   *
-   * In Mongo version 2.x, a client and a database handle were the same concept, but in 3.x
-   * they were separated. To handle everything with a single model, we treat them as the same here.
+   * Gets an import of MongoDB.
    */
-  private API::Node getAMongoClientOrDatabase() {
-    result = API::moduleImport("mongodb").getMember("MongoClient")
+  DataFlow::ModuleImportNode mongodb() { result.getPath() = "mongodb" }
+
+  /**
+   * Gets an access to `mongodb.MongoClient`.
+   */
+  private DataFlow::SourceNode getAMongoClient(DataFlow::TypeTracker t) {
+    t.start() and
+    result = mongodb().getAPropertyRead("MongoClient")
     or
-    result = getAMongoClientOrDatabase().getMember("db").getReturn()
+    exists(DataFlow::TypeTracker t2 | result = getAMongoClient(t2).track(t2, t))
+  }
+
+  /**
+   * Gets an access to `mongodb.MongoClient`.
+   */
+  DataFlow::SourceNode getAMongoClient() { result = getAMongoClient(DataFlow::TypeTracker::end()) }
+
+  /** Gets a data flow node that leads to a `connect` callback. */
+  private DataFlow::SourceNode getAMongoDbCallback(DataFlow::TypeBackTracker t) {
+    t.start() and
+    result = getAMongoClient().getAMemberCall("connect").getArgument(1).getALocalSource()
     or
-    result = getAMongoClientOrDatabase().getMember("connect").getLastParameter().getParameter(1)
+    exists(DataFlow::TypeBackTracker t2 | result = getAMongoDbCallback(t2).backtrack(t2, t))
+  }
+
+  /** Gets a data flow node that leads to a `connect` callback. */
+  private DataFlow::FunctionNode getAMongoDbCallback() {
+    result = getAMongoDbCallback(DataFlow::TypeBackTracker::end())
+  }
+
+  /**
+   * Gets an expression that may refer to a MongoDB database connection.
+   */
+  private DataFlow::SourceNode getAMongoDb(DataFlow::TypeTracker t) {
+    t.start() and
+    result = getAMongoDbCallback().getParameter(1)
+    or
+    exists(DataFlow::TypeTracker t2 | result = getAMongoDb(t2).track(t2, t))
+  }
+
+  /**
+   * Gets an expression that may refer to a MongoDB database connection.
+   */
+  DataFlow::SourceNode getAMongoDb() { result = getAMongoDb(DataFlow::TypeTracker::end()) }
+
+  /**
+   * A data flow node that may hold a MongoDB collection.
+   */
+  abstract class Collection extends DataFlow::SourceNode { }
+
+  /**
+   * A collection resulting from calling `Db.collection(...)`.
+   */
+  private class CollectionFromDb extends Collection {
+    CollectionFromDb() {
+      this = getAMongoDb().getAMethodCall("collection")
+      or
+      this = getAMongoDb().getAMethodCall("collection").getCallback(1).getParameter(0)
+    }
+  }
+
+  /**
+   * A collection based on the type `mongodb.Collection`.
+   *
+   * Note that this also covers `mongoose` models since they are subtypes
+   * of `mongodb.Collection`.
+   */
+  private class CollectionFromType extends Collection {
+    CollectionFromType() { hasUnderlyingType("mongodb", "Collection") }
   }
 
   /** Gets a data flow node referring to a MongoDB collection. */
-  private API::Node getACollection() {
-    // A collection resulting from calling `Db.collection(...)`.
-    exists(API::Node collection |
-      collection = getAMongoClientOrDatabase().getMember("collection").getReturn()
-    |
-      result = collection
-      or
-      result = collection.getParameter(1).getParameter(0)
-    )
+  private DataFlow::SourceNode getACollection(DataFlow::TypeTracker t) {
+    t.start() and
+    result instanceof Collection
     or
-    // note that this also covers `mongoose` models since they are subtypes of `mongodb.Collection`
-    result = API::Node::ofType("mongodb", "Collection")
+    exists(DataFlow::TypeTracker t2 | result = getACollection(t2).track(t2, t))
   }
 
+  /** Gets a data flow node referring to a MongoDB collection. */
+  DataFlow::SourceNode getACollection() { result = getACollection(DataFlow::TypeTracker::end()) }
+
   /** A call to a MongoDB query method. */
-  private class QueryCall extends DatabaseAccess, API::CallNode {
+  private class QueryCall extends DatabaseAccess, DataFlow::MethodCallNode {
     int queryArgIdx;
 
     QueryCall() {
-      exists(string method |
-        CollectionMethodSignatures::interpretsArgumentAsQuery(method, queryArgIdx) and
-        this = getACollection().getMember(method).getACall()
+      exists(string m | this = getACollection().getAMethodCall(m) |
+        m = "count" and queryArgIdx = 0
+        or
+        m = "distinct" and queryArgIdx = 1
+        or
+        m = "find" and queryArgIdx = 0
       )
     }
 
-    override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(this.getParameter(queryArgIdx))
-    }
+    override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
   }
 
   /**
    * An expression that is interpreted as a MongoDB query.
    */
-  class Query extends NoSql::Query {
-    QueryCall qc;
-
-    Query() { this = qc.getAQueryArgument().asExpr() }
-
-    override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
-  }
-
-  /**
-   * Provides signatures for the Collection methods.
-   */
-  module CollectionMethodSignatures {
-    /**
-     * Holds if Collection method `name` interprets parameter `n` as a query.
-     */
-    predicate interpretsArgumentAsQuery(string name, int n) {
-      // FilterQuery
-      (
-        name = "aggregate" and n = 0
-        or
-        name = "count" and n = 0
-        or
-        name = "countDocuments" and n = 0
-        or
-        name = "deleteMany" and n = 0
-        or
-        name = "deleteOne" and n = 0
-        or
-        name = "distinct" and n = 1
-        or
-        name = "find" and n = 0
-        or
-        name = "findOne" and n = 0
-        or
-        name = "findOneAndDelete" and n = 0
-        or
-        name = "findOneAndRemove" and n = 0
-        or
-        name = "findOneAndReplace" and n = 0
-        or
-        name = "findOneAndUpdate" and n = 0
-        or
-        name = "remove" and n = 0
-        or
-        name = "replaceOne" and n = 0
-        or
-        name = "update" and n = 0
-        or
-        name = "updateMany" and n = 0
-        or
-        name = "updateOne" and n = 0
-      )
-      or
-      // UpdateQuery
-      (
-        name = "findOneAndUpdate" and n = 1
-        or
-        name = "update" and n = 1
-        or
-        name = "updateMany" and n = 1
-        or
-        name = "updateOne" and n = 1
-      )
-    }
+  class Query extends NoSQL::Query {
+    Query() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
   }
 }
 
@@ -156,342 +132,20 @@ private module Mongoose {
   /**
    * Gets an import of Mongoose.
    */
-  API::Node getAMongooseInstance() { result = API::moduleImport("mongoose") }
+  DataFlow::ModuleImportNode getAMongooseInstance() { result.getPath() = "mongoose" }
 
   /**
-   * Gets a reference to `mongoose.createConnection`.
+   * Gets a call to `mongoose.createConnection`.
    */
-  API::Node createConnection() { result = getAMongooseInstance().getMember("createConnection") }
-
-  /**
-   * A Mongoose function.
-   */
-  abstract private class MongooseFunction extends API::Node {
-    /**
-     * Gets the API-graph node for the result from this function (if the function returns a `Query`).
-     */
-    abstract API::Node getQueryReturn();
-
-    /**
-     * Holds if this function returns a `Query` that evaluates to one or
-     * more Documents (`asArray` is false if it evaluates to a single
-     * Document).
-     */
-    abstract predicate returnsDocumentQuery(boolean asArray);
-
-    /**
-     * Gets an argument that this function interprets as a query.
-     */
-    abstract API::Node getQueryArgument();
+  DataFlow::CallNode createConnection() {
+    result = getAMongooseInstance().getAMemberCall("createConnection")
   }
 
   /**
-   * Provides classes modeling the Mongoose Model class
+   * A Mongoose collection object.
    */
-  module Model {
-    private class ModelFunction extends MongooseFunction {
-      string methodName;
-
-      ModelFunction() { this = getModelObject().getMember(methodName) }
-
-      override API::Node getQueryReturn() {
-        MethodSignatures::returnsQuery(methodName) and result = this.getReturn()
-      }
-
-      override predicate returnsDocumentQuery(boolean asArray) {
-        MethodSignatures::returnsDocumentQuery(methodName, asArray)
-      }
-
-      override API::Node getQueryArgument() {
-        exists(int n |
-          MethodSignatures::interpretsArgumentAsQuery(methodName, n) and
-          result = this.getParameter(n)
-        )
-      }
-    }
-
-    /**
-     * Gets a API-graph node referring to a Mongoose Model object.
-     */
-    private API::Node getModelObject() {
-      result = getAMongooseInstance().getMember("model").getReturn()
-      or
-      exists(API::Node conn | conn = createConnection().getReturn() |
-        result = conn.getMember("model").getReturn() or
-        result = conn.getMember("models").getAMember()
-      )
-      or
-      result = API::Node::ofType("mongoose", "Model")
-    }
-
-    /**
-     * Provides signatures for the Model methods.
-     */
-    module MethodSignatures {
-      /**
-       * Holds if Model method `name` interprets parameter `n` as a query.
-       */
-      predicate interpretsArgumentAsQuery(string name, int n) {
-        // implement lots of the MongoDB collection interface
-        MongoDB::CollectionMethodSignatures::interpretsArgumentAsQuery(name, n)
-        or
-        name = "find" + ["ById", "One"] + "AndUpdate" and n = 1
-        or
-        name in ["delete" + ["Many", "One"], "geoSearch", "remove", "replaceOne", "where"] and
-        n = 0
-        or
-        name in [
-            "find" + ["", "ById", "One"],
-            "find" + ["ById", "One"] + "And" + ["Delete", "Remove", "Update"],
-            "update" + ["", "Many", "One"]
-          ] and
-        n = 0
-      }
-
-      /**
-       * Holds if Model method `name` returns a Query.
-       */
-      predicate returnsQuery(string name) {
-        name =
-          [
-            "$where", "count", "findOne", "findOneAndDelete", "findOneAndRemove",
-            "findOneAndReplace", "findOneAndUpdate", "geosearch", "remove", "replaceOne", "update",
-            "updateMany", "countDocuments", "updateOne", "where", "deleteMany", "deleteOne", "find",
-            "findById", "findByIdAndDelete", "findByIdAndRemove", "findByIdAndUpdate"
-          ]
-      }
-
-      /**
-       * Holds if Document method `name` returns a query that results in
-       * one or more documents, the documents are wrapped in an array
-       * if `asArray` is true.
-       */
-      predicate returnsDocumentQuery(string name, boolean asArray) {
-        asArray = false and name = "findOne"
-        or
-        asArray = true and name = "find"
-      }
-    }
-  }
-
-  /**
-   * Provides classes modeling the Mongoose Query class
-   */
-  module Query {
-    private class QueryFunction extends MongooseFunction {
-      string methodName;
-
-      QueryFunction() { this = getAMongooseQuery().getMember(methodName) }
-
-      override API::Node getQueryReturn() {
-        MethodSignatures::returnsQuery(methodName) and result = this.getReturn()
-      }
-
-      override predicate returnsDocumentQuery(boolean asArray) {
-        MethodSignatures::returnsDocumentQuery(methodName, asArray)
-      }
-
-      override API::Node getQueryArgument() {
-        exists(int n |
-          MethodSignatures::interpretsArgumentAsQuery(methodName, n) and
-          result = this.getParameter(n)
-        )
-      }
-    }
-
-    private class NewQueryFunction extends MongooseFunction {
-      NewQueryFunction() { this = getAMongooseInstance().getMember("Query") }
-
-      override API::Node getQueryReturn() { result = this.getInstance() }
-
-      override predicate returnsDocumentQuery(boolean asArray) { none() }
-
-      override API::Node getQueryArgument() { result = this.getParameter(2) }
-    }
-
-    /**
-     * Gets a data flow node referring to a Mongoose query object.
-     */
-    API::Node getAMongooseQuery() {
-      result = any(MongooseFunction f).getQueryReturn()
-      or
-      result = API::Node::ofType("mongoose", "Query")
-      or
-      result =
-        getAMongooseQuery()
-            .getMember(any(string name | MethodSignatures::returnsQuery(name)))
-            .getReturn()
-    }
-
-    /**
-     * Provides signatures for the Query methods.
-     */
-    module MethodSignatures {
-      /**
-       * Holds if Query method `name` interprets parameter `n` as a query.
-       */
-      predicate interpretsArgumentAsQuery(string name, int n) {
-        n = 0 and
-        name =
-          [
-            "and", "count", "findOneAndReplace", "findOneAndUpdate", "merge", "nor", "or", "remove",
-            "replaceOne", "setQuery", "setUpdate", "update", "countDocuments", "updateMany",
-            "updateOne", "where", "deleteMany", "deleteOne", "elemMatch", "find", "findOne",
-            "findOneAndDelete", "findOneAndRemove"
-          ]
-        or
-        n = 1 and
-        name = ["distinct", "findOneAndUpdate", "update", "updateMany", "updateOne"]
-      }
-
-      /**
-       * Holds if Query method `name` returns a Query.
-       */
-      predicate returnsQuery(string name) {
-        name =
-          [
-            "$where", "J", "comment", "count", "countDocuments", "distinct", "elemMatch", "equals",
-            "error", "estimatedDocumentCount", "exists", "explain", "all", "find", "findById",
-            "findOne", "findOneAndRemove", "findOneAndUpdate", "geometry", "get", "gt", "gte",
-            "hint", "and", "in", "intersects", "lean", "limit", "lt", "lte", "map", "map",
-            "maxDistance", "maxTimeMS", "batchsize", "maxscan", "mod", "ne", "near", "nearSphere",
-            "nin", "or", "orFail", "polygon", "populate", "box", "read", "readConcern", "regexp",
-            "remove", "select", "session", "set", "setOptions", "setQuery", "setUpdate", "center",
-            "size", "skip", "slaveOk", "slice", "snapshot", "sort", "update", "w", "where",
-            "within", "centerSphere", "wtimeout", "circle", "collation"
-          ]
-      }
-
-      /**
-       * Holds if Query method `name` returns a query that results in
-       * one or more documents, the documents are wrapped in an array
-       * if `asArray` is true.
-       */
-      predicate returnsDocumentQuery(string name, boolean asArray) {
-        asArray = false and name = "findOne"
-        or
-        asArray = true and name = "find"
-      }
-    }
-  }
-
-  /**
-   * Provides classes modeling the Mongoose Document class
-   */
-  module Document {
-    private class DocumentFunction extends MongooseFunction {
-      string methodName;
-
-      DocumentFunction() { this = getAMongooseDocument().getMember(methodName) }
-
-      override API::Node getQueryReturn() {
-        MethodSignatures::returnsQuery(methodName) and result = this.getReturn()
-      }
-
-      override predicate returnsDocumentQuery(boolean asArray) {
-        MethodSignatures::returnsDocumentQuery(methodName, asArray)
-      }
-
-      override API::Node getQueryArgument() {
-        exists(int n |
-          MethodSignatures::interpretsArgumentAsQuery(methodName, n) and
-          result = this.getParameter(n)
-        )
-      }
-    }
-
-    /**
-     * A Mongoose Document that is retrieved from the backing database.
-     */
-    class RetrievedDocument extends API::Node {
-      RetrievedDocument() {
-        exists(boolean asArray, API::Node param |
-          exists(MongooseFunction func |
-            func.returnsDocumentQuery(asArray) and
-            param = func.getLastParameter().getParameter(1)
-          )
-          or
-          exists(API::Node f |
-            f = Query::getAMongooseQuery().getMember("then") and
-            param = f.getParameter(0).getParameter(0)
-            or
-            f = Query::getAMongooseQuery().getMember("exec") and
-            param = f.getParameter(0).getParameter(1)
-          |
-            exists(DataFlow::MethodCallNode pred |
-              // limitation: look at the previous method call	
-              Query::MethodSignatures::returnsDocumentQuery(pred.getMethodName(), asArray) and
-              pred.getAMethodCall() = f.getACall()
-            )
-          )
-        |
-          asArray = false and this = param
-          or
-          asArray = true and
-          // limitation: look for direct accesses
-          this = param.getUnknownMember()
-        )
-      }
-    }
-
-    /**
-     * Gets a data flow node referring to a Mongoose Document object.
-     */
-    private API::Node getAMongooseDocument() {
-      result instanceof RetrievedDocument
-      or
-      result = API::Node::ofType("mongoose", "Document")
-      or
-      result =
-        getAMongooseDocument()
-            .getMember(any(string name | MethodSignatures::returnsDocument(name)))
-            .getReturn()
-    }
-
-    private module MethodSignatures {
-      /**
-       * Holds if Document method `name` returns a Query.
-       */
-      predicate returnsQuery(string name) {
-        // Documents are subtypes of Models
-        Model::MethodSignatures::returnsQuery(name) or
-        name = "replaceOne" or
-        name = "update" or
-        name = "updateOne"
-      }
-
-      /**
-       * Holds if Document method `name` interprets parameter `n` as a query.
-       */
-      predicate interpretsArgumentAsQuery(string name, int n) {
-        // Documents are subtypes of Models
-        Model::MethodSignatures::interpretsArgumentAsQuery(name, n)
-        or
-        n = 0 and
-        (
-          name = "replaceOne" or
-          name = "update" or
-          name = "updateOne"
-        )
-      }
-
-      /**
-       * Holds if Document method `name` returns a query that results in
-       * one or more documents, the documents are wrapped in an array
-       * if `asArray` is true.
-       */
-      predicate returnsDocumentQuery(string name, boolean asArray) {
-        // Documents are subtypes of Models
-        Model::MethodSignatures::returnsDocumentQuery(name, asArray)
-      }
-
-      /**
-       * Holds if Document method `name` returns a Document.
-       */
-      predicate returnsDocument(string name) {
-        name = ["depopulate", "init", "populate", "overwrite"]
-      }
-    }
+  class Model extends MongoDB::Collection {
+    Model() { this = getAMongooseInstance().getAMemberCall("model") }
   }
 
   /**
@@ -501,9 +155,7 @@ private module Mongoose {
     string kind;
 
     Credentials() {
-      exists(string prop |
-        this = createConnection().getParameter(3).getMember(prop).getARhs().asExpr()
-      |
+      exists(string prop | this = createConnection().getOptionArgument(3, prop).asExpr() |
         prop = "user" and kind = "user name"
         or
         prop = "pass" and kind = "password"
@@ -512,308 +164,4 @@ private module Mongoose {
 
     override string getCredentialsKind() { result = kind }
   }
-
-  /**
-   * An expression that is interpreted as a (part of a) MongoDB query.
-   */
-  class MongoDBQueryPart extends NoSql::Query {
-    MongooseFunction f;
-
-    MongoDBQueryPart() { this = f.getQueryArgument().getARhs().asExpr() }
-
-    override DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(f.getQueryArgument())
-    }
-  }
-
-  /**
-   * An evaluation of a MongoDB query.
-   */
-  class ShorthandQueryEvaluation extends DatabaseAccess, DataFlow::InvokeNode {
-    MongooseFunction f;
-
-    ShorthandQueryEvaluation() {
-      this = f.getACall() and
-      // shorthand for execution: provide a callback
-      exists(f.getQueryReturn()) and
-      exists(this.getCallback(this.getNumArgument() - 1))
-    }
-
-    override DataFlow::Node getAQueryArgument() {
-      // NB: the complete information is not easily accessible for deeply chained calls
-      f.getQueryArgument().getARhs() = result
-    }
-
-    override DataFlow::Node getAResult() {
-      result = this.getCallback(this.getNumArgument() - 1).getParameter(1)
-    }
-  }
-
-  class ExplicitQueryEvaluation extends DatabaseAccess, DataFlow::CallNode {
-    string member;
-
-    ExplicitQueryEvaluation() {
-      // explicit execution using a Query method call
-      member = ["exec", "then", "catch"] and
-      Query::getAMongooseQuery().getMember(member).getACall() = this
-    }
-
-    private int resultParamIndex() {
-      member = "then" and result = 0
-      or
-      member = "exec" and result = 1
-    }
-
-    override DataFlow::Node getAResult() {
-      result = this.getCallback(_).getParameter(this.resultParamIndex())
-    }
-
-    override DataFlow::Node getAQueryArgument() {
-      // NB: the complete information is not easily accessible for deeply chained calls
-      none()
-    }
-  }
-}
-
-/**
- * Provides classes modeling the Minimongo library.
- */
-private module Minimongo {
-  /**
-   * Provides signatures for the Collection methods.
-   */
-  module CollectionMethodSignatures {
-    /**
-     * Holds if Collection method `name` interprets parameter `n` as a query.
-     */
-    predicate interpretsArgumentAsQuery(string m, int queryArgIdx) {
-      // implements most of the MongoDB interface
-      MongoDB::CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
-    }
-  }
-
-  /** A call to a Minimongo query method. */
-  private class QueryCall extends DatabaseAccess, API::CallNode {
-    int queryArgIdx;
-
-    QueryCall() {
-      exists(string m |
-        this =
-          API::moduleImport("minimongo")
-              .getAMember()
-              .getReturn()
-              .getAMember()
-              .getMember(m)
-              .getACall() and
-        CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
-      )
-    }
-
-    override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(this.getParameter(queryArgIdx))
-    }
-  }
-
-  /**
-   * An expression that is interpreted as a Minimongo query.
-   */
-  class Query extends NoSql::Query {
-    QueryCall qc;
-
-    Query() { this = qc.getAQueryArgument().asExpr() }
-
-    override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
-  }
-}
-
-/**
- * Provides classes modeling the MarsDB library.
- */
-private module MarsDB {
-  private class MarsDBAccess extends DatabaseAccess, DataFlow::CallNode {
-    string method;
-
-    MarsDBAccess() {
-      this =
-        API::moduleImport("marsdb")
-            .getMember("Collection")
-            .getInstance()
-            .getMember(method)
-            .getACall()
-    }
-
-    string getMethod() { result = method }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    override DataFlow::Node getAQueryArgument() { none() }
-  }
-
-  /** A call to a MarsDB query method. */
-  private class QueryCall extends MarsDBAccess, API::CallNode {
-    int queryArgIdx;
-
-    QueryCall() {
-      exists(string m |
-        this.getMethod() = m and
-        // implements parts of the Minimongo interface
-        Minimongo::CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
-      )
-    }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
-
-    DataFlow::Node getACodeOperator() {
-      result = getADollarWhereProperty(this.getParameter(queryArgIdx))
-    }
-  }
-
-  /**
-   * An expression that is interpreted as a MarsDB query.
-   */
-  class Query extends NoSql::Query {
-    QueryCall qc;
-
-    Query() { this = qc.getAQueryArgument().asExpr() }
-
-    override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
-  }
-}
-
-/**
- * Provides classes modeling the `Node Redis` library.
- *
- * Redis is an in-memory key-value store and not a database,
- * but `Node Redis` can be exploited similarly to a NoSQL database by giving a method an array as argument instead of a string.
- * As an example the below two invocations of `client.set` are equivalent:
- *
- * ```
- * const redis = require("redis");
- * const client = redis.createClient();
- * client.set("key", "value");
- * client.set(["key", "value"]);
- * ```
- *
- * ioredis is a very similar library. However, ioredis does not support array arguments in the same way, and is therefore not vulnerable to the same kind of type confusion.
- */
-private module Redis {
-  /**
-   * Gets a `Node Redis` client.
-   */
-  private API::Node client() {
-    result = API::moduleImport("redis").getMember("createClient").getReturn()
-    or
-    result = API::moduleImport("redis").getMember("RedisClient").getInstance()
-    or
-    result = client().getMember("duplicate").getReturn()
-    or
-    result = client().getMember("duplicate").getLastParameter().getParameter(1)
-  }
-
-  /**
-   * Gets a (possibly chained) reference to a batch operation object.
-   * These have the same API as a redis client, except the calls are chained, and the sequence is terminated with a `.exec` call.
-   */
-  private API::Node multi() {
-    result = client().getMember(["multi", "batch"]).getReturn()
-    or
-    result = multi().getAMember().getReturn()
-  }
-
-  /**
-   * Gets a `Node Redis` client instance. Either a client created using `createClient()`, or a batch operation object.
-   */
-  private API::Node redis() { result = [client(), multi()] }
-
-  /**
-   * Provides signatures for the query methods from Node Redis.
-   */
-  module QuerySignatures {
-    /**
-     * Holds if `method` interprets parameter `argIndex` as a key, and a later parameter determines a value/field.
-     * Thereby the method is vulnerable if parameter `argIndex` is unexpectedly an array instead of a string, as an attacker can control arguments to Redis that the attacker was not supposed to control.
-     *
-     * Only setters and similar methods are included.
-     * For getter-like methods it is not generally possible to gain access "outside" of where you are supposed to have access,
-     * it is at most possible to get a Redis call to return more results than expected (e.g. by adding more members to [`geohash`](https://redis.io/commands/geohash)).
-     */
-    predicate argumentIsAmbiguousKey(string method, int argIndex) {
-      method =
-        [
-          "set", "publish", "append", "bitfield", "decrby", "getset", "hincrby", "hincrbyfloat",
-          "hset", "hsetnx", "incrby", "incrbyfloat", "linsert", "lpush", "lpushx", "lset", "ltrim",
-          "rename", "renamenx", "rpushx", "setbit", "setex", "smove", "zincrby", "zinterstore",
-          "hdel", "lpush", "pfadd", "rpush", "sadd", "sdiffstore", "srem"
-        ] and
-      argIndex = 0
-      or
-      method = ["bitop", "hmset", "mset", "msetnx", "geoadd"] and
-      argIndex in [0 .. any(DataFlow::InvokeNode invk).getNumArgument() - 1]
-    }
-  }
-
-  /**
-   * An expression that is interpreted as a key in a Node Redis call.
-   */
-  class RedisKeyArgument extends NoSql::Query {
-    RedisKeyArgument() {
-      exists(string method, int argIndex |
-        QuerySignatures::argumentIsAmbiguousKey(method, argIndex) and
-        this = redis().getMember(method).getParameter(argIndex).getARhs().asExpr()
-      )
-    }
-  }
-
-  /**
-   * An access to a database through redis
-   */
-  class RedisDatabaseAccess extends DatabaseAccess, DataFlow::CallNode {
-    RedisDatabaseAccess() { this = redis().getMember(_).getACall() }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    override DataFlow::Node getAQueryArgument() { none() }
-  }
-}
-
-/**
- * Provides classes modeling the `ioredis` library.
- *
- * ```
- * import Redis from 'ioredis'
- * let client = new Redis(...)
- * ```
- */
-private module IoRedis {
-  /**
-   * Gets an `ioredis` client.
-   */
-  API::Node ioredis() { result = API::moduleImport("ioredis").getInstance() }
-
-  /**
-   * An access to a database through ioredis
-   */
-  class IoRedisDatabaseAccess extends DatabaseAccess, DataFlow::CallNode {
-    IoRedisDatabaseAccess() { this = ioredis().getMember(_).getACall() }
-
-    override DataFlow::Node getAResult() {
-      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
-    }
-
-    override DataFlow::Node getAQueryArgument() { none() }
-  }
 }

javascript/ql/lib/semmle/javascript/frameworks/NodeJSLib.qll

@@ -493,56 +493,11 @@ module NodeJSLib {
    */
   module FS {
     /**
-     * Gets a member `member` from module `fs` or its drop-in replacements `graceful-fs`, `fs-extra`, `original-fs`.
+     * A member `member` from module `fs`.
      */
     DataFlow::SourceNode moduleMember(string member) {
-      result = fsModule(DataFlow::TypeTracker::end()).getAPropertyRead(member)
-    }
-
-    private DataFlow::SourceNode fsModule(DataFlow::TypeTracker t) {
-      exists(string moduleName |
-        moduleName = ["mz/fs", "original-fs", "fs-extra", "graceful-fs", "fs"]
-      |
-        result = DataFlow::moduleImport(moduleName)
-        or
-        // extra support for flexible names
-        result.asExpr().(Require).getArgument(0).mayHaveStringValue(moduleName)
-      ) and
-      t.start()
-      or
-      t.start() and
-      result = DataFlow::moduleMember("fs", "promises")
-      or
-      exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = fsModule(t2) |
-        result = pred.track(t2, t)
-        or
-        t.continue() = t2 and
-        exists(Promisify::PromisifyAllCall promisifyAllCall |
-          result = promisifyAllCall and
-          pred.flowsTo(promisifyAllCall.getArgument(0))
-        )
-        or
-        // const fs = require('fs');
-        // let fs_copy = methods.reduce((obj, method) => {
-        //  obj[method] = fs[method];
-        //  return obj;
-        // }, {});
-        t.continue() = t2 and
-        exists(
-          DataFlow::MethodCallNode call, DataFlow::ParameterNode obj, DataFlow::SourceNode method
-        |
-          call.getMethodName() = "reduce" and
-          result = call and
-          obj = call.getABoundCallbackParameter(0, 0) and
-          obj.flowsTo(any(DataFlow::FunctionNode f).getAReturn()) and
-          exists(DataFlow::PropWrite write, DataFlow::PropRead read |
-            write = obj.getAPropertyWrite() and
-            method.flowsToExpr(write.getPropertyNameExpr()) and
-            method.flowsToExpr(read.getPropertyNameExpr()) and
-            read.getBase().getALocalSource() = fsModule(t2) and
-            write.getRhs() = maybePromisified(read)
-          )
-        )
+      exists(string moduleName | moduleName = ["fs"] |
+        result = DataFlow::moduleMember(moduleName, member)
       )
     }
   }
@@ -553,7 +508,7 @@ module NodeJSLib {
   private class NodeJSFileSystemAccess extends FileSystemAccess, DataFlow::CallNode {
     string methodName;
 
-    NodeJSFileSystemAccess() { this = maybePromisified(FS::moduleMember(methodName)).getACall() }
+    NodeJSFileSystemAccess() { this = FS::moduleMember(methodName).getACall() }
 
     /**
      * Gets the name of the called method.