Merge branch 'main' into rdmarsh/cpp/use-taint-configuration-dtt

Author: MathiasVP

CONTRIBUTING.md

@@ -38,7 +38,7 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html).
 
-    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/install-pre-commit-hook.md) for instructions on how to install the hook.
+    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/pre-commit-hook-setup.md) for instructions on how to install the hook.
 
 4. **Compilation**
 

cpp/change-notes/2021-03-11-failed-extractions.md

@@ -0,0 +1,2 @@
+codescanning
+* Added cpp/diagnostics/failed-extractions. This query gives information about which extractions did not run to completion.

cpp/ql/src/Diagnostics/FailedExtractions.ql

@@ -0,0 +1,22 @@
+/**
+ * @name Failed extractions
+ * @description Gives the command-line of compilations for which extraction did not run to completion.
+ * @kind diagnostic
+ * @id cpp/diagnostics/failed-extractions
+ */
+
+import cpp
+
+class AnonymousCompilation extends Compilation {
+  override string toString() { result = "<compilation>" }
+}
+
+string describe(Compilation c) {
+  if c.getArgument(1) = "--mimic"
+  then result = "compiler invocation " + concat(int i | i > 1 | c.getArgument(i), " " order by i)
+  else result = "extractor invocation " + concat(int i | | c.getArgument(i), " " order by i)
+}
+
+from Compilation c
+where not c.normalTermination()
+select c, "Extraction failed for " + describe(c), 2

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

@@ -276,7 +276,10 @@ class File extends Container, @file {
       c.getAFileCompiled() = this and
       (
         c.getAnArgument() = "--microsoft" or
-        c.getAnArgument().toLowerCase().replaceAll("\\", "/").matches("%/cl.exe")
+        c.getAnArgument()
+            .toLowerCase()
+            .replaceAll("\\", "/")
+            .matches(["%/cl.exe", "%/clang-cl.exe"])
       )
     )
     or

docs/codeql/writing-codeql-queries/troubleshooting-query-performance.rst

@@ -12,7 +12,7 @@ This topic offers some simple tips on how to avoid common problems that can affe
 Before reading the tips below, it is worth reiterating a few important points about CodeQL and the QL language:
 
 - CodeQL :ref:`predicates <predicates>` and :ref:`classes <classes>` are evaluated to database `tables <https://en.wikipedia.org/wiki/Table_(database)>`__. Large predicates generate large tables with many rows, and are therefore expensive to compute.
-- The QL language is implemented using standard database operations and `relational algebra <https://en.wikipedia.org/wiki/Relational_algebra>`__ (such as join, projection, and union). For more information about query languages and databases, see ":ref:`About the QL language <about-the-ql-language>`.
+- The QL language is implemented using standard database operations and `relational algebra <https://en.wikipedia.org/wiki/Relational_algebra>`__ (such as join, projection, and union). For more information about query languages and databases, see ":ref:`About the QL language <about-the-ql-language>`."
 - Queries are evaluated *bottom-up*, which means that a predicate is not evaluated until *all* of the predicates that it depends on are evaluated. For more information on query evaluation, see ":ref:`Evaluation of QL programs <evaluation-of-ql-programs>`." 
 
 Performance tips

java/change-notes/2021-03-05-commons-object-utils.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Added models for `ObjectUtils` methods in the Apache Commons Lang library. This may lead to more results from any dataflow query where traversal of `ObjectUtils` methods means we can now complete a path from a source of tainted data to a corresponding sink.

java/ql/src/semmle/code/java/Modules.qll

@@ -144,7 +144,7 @@ class OpensDirective extends Directive, @opens {
 
   /**
    * Gets a module specified in the `to` clause of this
-   * `exports` directive, if any.
+   * `opens` directive, if any.
    */
   Module getATargetModule() { opensTo(this, result) }
 

java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -46,7 +46,8 @@ predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   localAdditionalTaintUpdateStep(src.asExpr(),
     sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
   or
-  summaryStep(src, sink, "taint")
+  summaryStep(src, sink, "taint") and
+  not summaryStep(src, sink, "value")
   or
   exists(Argument arg |
     src.asExpr() = arg and

java/ql/src/semmle/code/java/frameworks/apache/Lang.qll

@@ -396,3 +396,30 @@ private class ApacheRegExUtilsModel extends SummaryModelCsv {
       ]
   }
 }
+
+/**
+ * Taint-propagating models for `ObjectUtils`.
+ */
+private class ApacheObjectUtilsModel extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        // Note all the functions annotated with `taint` flow really should have `value` flow,
+        // but we don't support value-preserving varargs functions at the moment.
+        "org.apache.commons.lang3;ObjectUtils;false;clone;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;cloneIfPossible;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;CONST;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;CONST_BYTE;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;CONST_SHORT;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;defaultIfNull;;;Argument;ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;firstNonNull;;;Argument;ReturnValue;taint",
+        "org.apache.commons.lang3;ObjectUtils;false;getIfNull;;;Argument[0];ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;max;;;Argument;ReturnValue;taint",
+        "org.apache.commons.lang3;ObjectUtils;false;median;;;Argument;ReturnValue;taint",
+        "org.apache.commons.lang3;ObjectUtils;false;min;;;Argument;ReturnValue;taint",
+        "org.apache.commons.lang3;ObjectUtils;false;mode;;;Argument;ReturnValue;taint",
+        "org.apache.commons.lang3;ObjectUtils;false;requireNonEmpty;;;Argument[0];ReturnValue;value",
+        "org.apache.commons.lang3;ObjectUtils;false;toString;(Object,String);;Argument[1];ReturnValue;value"
+      ]
+  }
+}

java/ql/test/library-tests/frameworks/apache-commons-lang3/ObjectUtilsTest.java

@@ -0,0 +1,41 @@
+import org.apache.commons.lang3.ObjectUtils;
+
+public class ObjectUtilsTest {
+  String taint() { return "tainted"; }
+
+  private static class IntSource {
+    static int taint() { return 0; }
+  }
+
+  void sink(Object o) {}
+
+  void test() throws Exception {
+    sink(ObjectUtils.clone(taint())); // $hasValueFlow
+    sink(ObjectUtils.cloneIfPossible(taint())); // $hasValueFlow
+    sink(ObjectUtils.CONST(taint())); // $hasValueFlow
+    sink(ObjectUtils.CONST_SHORT(IntSource.taint())); // $hasValueFlow
+    sink(ObjectUtils.CONST_BYTE(IntSource.taint())); // $hasValueFlow
+    sink(ObjectUtils.defaultIfNull(taint(), null)); // $hasValueFlow
+    sink(ObjectUtils.defaultIfNull(null, taint())); // $hasValueFlow
+    sink(ObjectUtils.firstNonNull(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.firstNonNull(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.firstNonNull(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.getIfNull(taint(), null)); // $hasValueFlow
+    sink(ObjectUtils.max(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.max(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.max(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.median(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.median((String)null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.median((String)null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.min(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.min(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.min(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.mode(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.mode(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.mode(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow
+    sink(ObjectUtils.requireNonEmpty(taint(), "message")); // $hasValueFlow
+    sink(ObjectUtils.requireNonEmpty("not null", taint())); // GOOD (message doesn't propagate to the return)
+    sink(ObjectUtils.toString(taint(), "default string")); // GOOD (first argument is stringified)
+    sink(ObjectUtils.toString(null, taint())); // $hasValueFlow
+  }
+}