Merge branch 'master' into sideeffect

Author: geoffw0

change-notes/1.24/analysis-csharp.md

@@ -21,6 +21,7 @@ The following changes in version 1.24 affect C# analysis in all applications.
 | Potentially dangerous use of non-short-circuit logic (`cs/non-short-circuit`) | Fewer false positive results | Results have been removed when the expression contains an `out` parameter. |
 | Dereferenced variable may be null (`cs/dereferenced-value-may-be-null`) | More results | Results are reported from parameters with a default value of `null`. |
 | Useless assignment to local variable (`cs/useless-assignment-to-local`) | Fewer false positive results | Results have been removed when the value assigned is an (implicitly or explicitly) cast default-like value. For example, `var s = (string)null` and `string s = default`. |
+| XPath injection (`cs/xml/xpath-injection`) | More results | The query now recognizes calls to methods on `System.Xml.XPath.XPathNavigator` objects. |
 
 ## Removal of old queries
 

change-notes/1.24/analysis-javascript.md

@@ -6,16 +6,18 @@
 
 * Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
 
-* Imports with the `.js` extension can now be resolved to a TypeScript file,
+* Resolution of imports has improved, leading to more results from the security queries:
+  - Imports with the `.js` extension can now be resolved to a TypeScript file,
   when the import refers to a file generated by TypeScript.
+  - Imports that rely on path-mappings from a `tsconfig.json` file can now be resolved.
+  - Export declarations of the form `export * as ns from "x"` are now analyzed more precisely.
 
-* Imports that rely on path-mappings from a `tsconfig.json` file can now be resolved.
+* The analysis of sanitizers has improved, leading to more accurate results from the security queries.
+  In particular:
+  - Sanitizer guards now act across function boundaries in more cases.
+  - Sanitizers can now better distinguish between a tainted value and an object _containing_ a tainted value.
 
-* Export declarations of the form `export * as ns from "x"` are now analyzed more precisely.
-
-* The analysis of sanitizer guards has improved, leading to fewer false-positive results from the security queries.
-
-* The call graph construction has been improved, leading to more results from the security queries:
+* Call graph construction has been improved, leading to more results from the security queries:
   - Calls can now be resolved to indirectly-defined class members in more cases.
   - Calls through partial invocations such as `.bind` can now be resolved in more cases.
 
@@ -40,11 +42,14 @@
   - [ncp](https://www.npmjs.com/package/ncp)
   - [node-dir](https://www.npmjs.com/package/node-dir)
   - [path-exists](https://www.npmjs.com/package/path-exists)
+  - [pg](https://www.npmjs.com/package/pg)
   - [react](https://www.npmjs.com/package/react)
   - [recursive-readdir](https://www.npmjs.com/package/recursive-readdir)
   - [request](https://www.npmjs.com/package/request)
   - [rimraf](https://www.npmjs.com/package/rimraf)
   - [send](https://www.npmjs.com/package/send)
+  - [SockJS](https://www.npmjs.com/package/sockjs)
+  - [SockJS-client](https://www.npmjs.com/package/sockjs-client)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [vinyl-fs](https://www.npmjs.com/package/vinyl-fs)
   - [write-file-atomic](https://www.npmjs.com/package/write-file-atomic)
@@ -80,8 +85,14 @@
 | Use of password hash with insufficient computational effort (`js/insufficient-password-hash`) | Fewer false positive results | This query now recognizes additional cases that do not require secure hashing. |
 | Useless regular-expression character escape (`js/useless-regexp-character-escape`) | Fewer false positive results | This query now distinguishes escapes in strings and regular expression literals. |
 | Identical operands (`js/redundant-operation`) | Fewer results | This query now recognizes cases where the operands change a value using ++/-- expressions. |
+| Superfluous trailing arguments (`js/superfluous-trailing-arguments`) | Fewer results | This query now recognizes cases where a function uses the `Function.arguments` value to process a variable number of parameters. |
 
 ## Changes to libraries
 
 * The predicates `RegExpTerm.getSuccessor` and `RegExpTerm.getPredecessor` have been changed to reflect textual, not operational, matching order. This only makes a difference in lookbehind assertions, which are operationally matched backwards. Previously, `getSuccessor` would mimick this, so in an assertion `(?<=ab)` the term `b` would be considered the predecessor, not the successor, of `a`. Textually, however, `a` is still matched before `b`, and this is the order we now follow.
 * An extensible model of the `EventEmitter` pattern has been implemented.
+* Taint-tracking configurations now interact differently with the `data` flow label, which may affect queries
+  that combine taint-tracking and flow labels.
+  - Sources added by the 1-argument `isSource` predicate are associated with the `taint` label now, instead of the `data` label.
+  - Sanitizers now only block the `taint` label. As a result, sanitizers no longer block the flow of tainted values wrapped inside a property of an object.
+    To retain the old behavior, instead use a barrier, or block the `data` flow label using a labeled sanitizer.

change-notes/1.24/analysis-python.md

@@ -4,6 +4,8 @@ The following changes in version 1.24 affect Python analysis in all applications
 
 ## General improvements
 
+Support for Django version 2.x and 3.x
+
 ## New queries
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
@@ -13,6 +15,7 @@ The following changes in version 1.24 affect Python analysis in all applications
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
+| Uncontrolled command line (`py/command-line-injection`) | More results | We now model the `fabric` and `invoke` pacakges for command execution. |
 
 ### Web framework support
 

config/identical-files.json

@@ -39,6 +39,12 @@
     "java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
     "java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll"
   ],
+  "DataFlow Java/C++/C# Consistency checks": [
+    "java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll",
+    "cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll",
+    "csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll"
+  ],
   "C++ SubBasicBlocks": [
     "cpp/ql/src/semmle/code/cpp/controlflow/SubBasicBlocks.qll",
     "cpp/ql/src/semmle/code/cpp/dataflow/internal/SubBasicBlocks.qll"

cpp/ql/src/semmle/code/cpp/Function.qll

@@ -133,10 +133,16 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
    */
   Type getUnspecifiedType() { result = getType().getUnspecifiedType() }
 
-  /** Gets the nth parameter of this function. */
+  /**
+   * Gets the nth parameter of this function. There is no result for the
+   * implicit `this` parameter, and there is no `...` varargs pseudo-parameter.
+   */
   Parameter getParameter(int n) { params(unresolveElement(result), underlyingElement(this), n, _) }
 
-  /** Gets a parameter of this function. */
+  /**
+   * Gets a parameter of this function. There is no result for the implicit
+   * `this` parameter, and there is no `...` varargs pseudo-parameter.
+   */
   Parameter getAParameter() { params(unresolveElement(result), underlyingElement(this), _, _) }
 
   /**

cpp/ql/src/semmle/code/cpp/Variable.qll

@@ -397,16 +397,21 @@ class StaticStorageDurationVariable extends Variable {
  */
 private predicate runtimeExprInStaticInitializer(Expr e) {
   inStaticInitializer(e) and
-  if e instanceof AggregateLiteral
+  if e instanceof AggregateLiteral // in sync with the cast in `inStaticInitializer`
   then runtimeExprInStaticInitializer(e.getAChild())
   else not e.getFullyConverted().isConstant()
 }
 
-/** Holds if `e` is part of the initializer of a `StaticStorageDurationVariable`. */
+/**
+ * Holds if `e` is the initializer of a `StaticStorageDurationVariable`, either
+ * directly or below some top-level `AggregateLiteral`s.
+ */
 private predicate inStaticInitializer(Expr e) {
   exists(StaticStorageDurationVariable var | e = var.getInitializer().getExpr())
   or
-  inStaticInitializer(e.getParent())
+  // The cast to `AggregateLiteral` ensures we only compute what'll later be
+  // needed by `runtimeExprInStaticInitializer`.
+  inStaticInitializer(e.getParent().(AggregateLiteral))
 }
 
 /**