Merge branch 'master' into copymove

Author: geoffw0

.codeqlmanifest.json

@@ -1,5 +1,6 @@
 { "provide": [ "*/ql/src/qlpack.yml",
                "*/ql/test/qlpack.yml",
+               "*/ql/examples/qlpack.yml",
                "*/upgrades/qlpack.yml",
                "misc/legacy-support/*/qlpack.yml",
                "misc/suite-helpers/qlpack.yml" ] }

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "omnisharp.autoStart": false
+}

change-notes/1.25/analysis-csharp.md

@@ -28,27 +28,51 @@ The following changes in version 1.25 affect C# analysis in all applications.
   such as `A<int>.B`, no longer are considered unbound generics. (Such nested types do,
   however, still have relevant `.getSourceDeclaration()`s, for example `A<>.B`.)
 * The data-flow library has been improved, which affects most security queries by potentially
-  adding more results. Flow through methods now takes nested field reads/writes into account.
-  For example, the library is able to track flow from `"taint"` to `Sink()` via the method
-  `GetF2F1()` in
-  ```csharp
-  class C1
-  {
-      string F1;
-  }
-
-  class C2
-  {
-      C1 F2;
-  
-      string GetF2F1() => F2.F1; // Nested field read
-
-      void M()
-      {
-          F2 = new C1() { F1 = "taint" };
-          Sink(GetF2F1()); // NEW: "taint" reaches here
-      }
-  }
-  ```
+  adding more results:
+  - Flow through methods now takes nested field reads/writes into account.
+    For example, the library is able to track flow from `"taint"` to `Sink()` via the method
+    `GetF2F1()` in
+    ```csharp
+    class C1
+    {
+        string F1;
+    }
+
+    class C2
+    {
+        C1 F2;
+
+        string GetF2F1() => F2.F1; // Nested field read
+
+        void M()
+        {
+            F2 = new C1() { F1 = "taint" };
+            Sink(GetF2F1()); // NEW: "taint" reaches here
+        }
+    }
+    ```
+  - Flow through collections is now modeled precisely. For example, instead of modeling an array
+    store `a[i] = x` as a taint-step from `x` to `a`, we now model it as a data-flow step that
+    stores `x` into `a`. To get the value back out, a matching read step must be taken.
+
+    For source-code based data-flow analysis, the following constructs are modeled as stores into
+    collections:
+    - Direct array assignments, `a[i] = x`.
+    - Array initializers, `new [] { x }`.
+    - C# 6-style array initializers, `new C() { Array = { [i] = x } }`.
+    - Call arguments that match a `params` parameter, where the C# compiler creates an array under-the-hood.
+    - `yield return` statements.
+
+    The following source-code constructs read from a collection:
+    - Direct array reads, `a[i]`.
+    - `foreach` statements.
+
+    For calls out to library code, existing flow summaries have been refined to precisely
+    capture how they interact with collection contents. For example, a call to
+    `System.Collections.Generic.List<T>.Add(T)` stores the value of the argument into the
+    qualifier, and a call to `System.Collections.Generic.List<T>.get_Item(int)` (that is, an
+    indexer call) reads contents out of the qualifier. Moreover, the effect of
+    collection-clearing methods such as `System.Collections.Generic.List<T>.Clear()` is now
+    also modeled.
 
 ## Changes to autobuilder

change-notes/1.25/analysis-javascript.md

@@ -6,8 +6,10 @@
   - [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise)
   - [bluebird](http://bluebirdjs.com/)
   - [express](https://www.npmjs.com/package/express)
+  - [execa](https://www.npmjs.com/package/execa)
   - [fancy-log](https://www.npmjs.com/package/fancy-log)
   - [fastify](https://www.npmjs.com/package/fastify)
+  - [foreground-child](https://www.npmjs.com/package/foreground-child)
   - [fstream](https://www.npmjs.com/package/fstream)
   - [jGrowl](https://github.com/stanlemon/jGrowl)
   - [jQuery](https://jquery.com/)
@@ -17,6 +19,7 @@
   - [mssql](https://www.npmjs.com/package/mssql)
   - [mysql](https://www.npmjs.com/package/mysql)
   - [npmlog](https://www.npmjs.com/package/npmlog)
+  - [opener](https://www.npmjs.com/package/opener)
   - [pg](https://www.npmjs.com/package/pg)
   - [sequelize](https://www.npmjs.com/package/sequelize)
   - [spanner](https://www.npmjs.com/package/spanner)

cpp/ql/examples/qlpack.yml

@@ -0,0 +1,3 @@
+name: codeql-cpp-examples
+version: 0.0.0
+libraryPathDependencies: codeql-cpp

cpp/ql/src/codeql-suites/cpp-lgtm-full.qls

@@ -9,3 +9,10 @@
     tags contain:
       - ide-contextual-queries/local-definitions
       - ide-contextual-queries/local-references
+- query: Metrics/Dependencies/ExternalDependencies.ql
+- query: Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
+- query: Metrics/Files/FLinesOfCode.ql
+- query: Metrics/Files/FLinesOfCommentedOutCode.ql
+- query: Metrics/Files/FLinesOfComments.ql
+- query: Metrics/Files/FLinesOfDuplicatedCode.ql
+- query: Metrics/Files/FNumberOfTests.ql

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

@@ -197,7 +197,8 @@ class Element extends ElementBase {
     initialisers(underlyingElement(this), unresolveElement(result), _, _) or
     exprconv(unresolveElement(result), underlyingElement(this)) or
     param_decl_bind(underlyingElement(this), _, unresolveElement(result)) or
-    using_container(unresolveElement(result), underlyingElement(this))
+    using_container(unresolveElement(result), underlyingElement(this)) or
+    static_asserts(unresolveElement(this), _, _, _, underlyingElement(result))
   }
 
   /** Gets the closest `Element` enclosing this one. */
@@ -278,12 +279,12 @@ class StaticAssert extends Locatable, @static_assert {
   /**
    * Gets the expression which this static assertion ensures is true.
    */
-  Expr getCondition() { static_asserts(underlyingElement(this), unresolveElement(result), _, _) }
+  Expr getCondition() { static_asserts(underlyingElement(this), unresolveElement(result), _, _, _) }
 
   /**
    * Gets the message which will be reported by the compiler if this static assertion fails.
    */
-  string getMessage() { static_asserts(underlyingElement(this), _, result, _) }
+  string getMessage() { static_asserts(underlyingElement(this), _, result, _, _) }
 
-  override Location getLocation() { static_asserts(underlyingElement(this), _, _, result) }
+  override Location getLocation() { static_asserts(underlyingElement(this), _, _, result, _) }
 }

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

@@ -582,7 +582,7 @@ class TemplateVariable extends Variable {
  *   float a;
  * }
  *
- * template<type T>
+ * template<typename T>
  * void myTemplateFunction() {
  *   T b;
  * }

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

@@ -49,6 +49,18 @@ predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
   )
 }
 
+/**
+ * A standard function such as `vsprintf` that has an output parameter
+ * and a variable argument list of type `va_arg`.
+ */
+private predicate primitiveVariadicFormatterOutput(TopLevelFunction f, int outputParamIndex) {
+  // note: this might look like the regular expression in `primitiveVariadicFormatter`, but
+  // there is one important difference: the [fs] part is not optional, as these classify
+  // the `printf` variants that write to a buffer.
+  // Conveniently, these buffer parameters are all at index 0.
+  f.getName().regexpMatch("_?_?va?[fs]n?w?printf(_s)?(_p)?(_l)?") and outputParamIndex = 0
+}
+
 private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
   exists(FunctionCall fc, int i |
     variadicFormatter(fc.getTarget(), i) and
@@ -57,6 +69,26 @@ private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
   )
 }
 
+private predicate callsVariadicFormatterOutput(Function f, int outputParamIndex) {
+  exists(FunctionCall fc, int i |
+    fc.getEnclosingFunction() = f and
+    variadicFormatterOutput(fc.getTarget(), i) and
+    fc.getArgument(i) = f.getParameter(outputParamIndex).getAnAccess()
+  )
+}
+
+/**
+ * Holds if `f` is a function such as `vprintf` that takes variable argument list
+ * of type `va_arg` and writes formatted output to a buffer given as a parameter at
+ * index `outputParamIndex`, if any.
+ */
+private predicate variadicFormatterOutput(Function f, int outputParamIndex) {
+  primitiveVariadicFormatterOutput(f, outputParamIndex)
+  or
+  not f.isVarargs() and
+  callsVariadicFormatterOutput(f, outputParamIndex)
+}
+
 /**
  * Holds if `f` is a function such as `vprintf` that has a format parameter
  * (at `formatParamIndex`) and a variable argument list of type `va_arg`.
@@ -78,6 +110,8 @@ class UserDefinedFormattingFunction extends FormattingFunction {
   UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _) }
 
   override int getFormatParameterIndex() { callsVariadicFormatter(this, result) }
+
+  override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -699,13 +699,20 @@ predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)
  */
 class BarrierGuard extends IRGuardCondition {
   /** Override this predicate to hold if this guard validates `instr` upon evaluating to `b`. */
-  abstract predicate checks(Instruction instr, boolean b);
+  predicate checksInstr(Instruction instr, boolean b) { none() }
+
+  /** Override this predicate to hold if this guard validates `expr` upon evaluating to `b`. */
+  predicate checks(Expr e, boolean b) { none() }
 
   /** Gets a node guarded by this guard. */
   final Node getAGuardedNode() {
     exists(ValueNumber value, boolean edge |
+      (
+        this.checksInstr(value.getAnInstruction(), edge)
+        or
+        this.checks(value.getAnInstruction().getConvertedResultExpression(), edge)
+      ) and
       result.asInstruction() = value.getAnInstruction() and
-      this.checks(value.getAnInstruction(), edge) and
       this.controls(result.asInstruction().getBlock(), edge)
     )
   }