Expose invoke nodes for external API sinks

Python: Add support for API graphs

.github/workflows/codeql-analysis.yml

@@ -2,7 +2,13 @@ name: "Code scanning - action"
 
 on:
   push:
+    branches:
+      - main
+      - 'rc/*'
   pull_request:
+    branches:
+      - main
+      - 'rc/*'
   schedule:
     - cron: '0 9 * * 1'
 
@@ -14,16 +20,7 @@ jobs:
     steps:
     - name: Checkout repository
       uses: actions/checkout@v2
-      with:
-        # We must fetch at least the immediate parents so that if this is
-        # a pull request then we can checkout the head.
-        fetch-depth: 2
 
-    # If this run was triggered by a pull request event, then checkout
-    # the head of the pull request instead of the merge commit.
-    - run: git checkout HEAD^2
-      if: ${{ github.event_name == 'pull_request' }}
-      
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v1

.github/workflows/generate-query-help-docs.yml

@@ -0,0 +1,60 @@
+name: Generate CodeQL query help documentation using Sphinx
+
+on:
+  workflow_dispatch:
+      inputs:
+        description:
+          description: A description of the purpose of this job. For human consumption.
+          required: false
+  push:
+    branches:
+     - 'lgtm.com'
+  pull_request:
+    paths:
+      - '.github/workflows/generate-query-help-docs.yml'
+      - 'docs/codeql/query-help/**'
+      
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Clone github/codeql
+      uses: actions/checkout@v2
+      with:
+        path: codeql
+    - name: Clone github/codeql-go
+      uses: actions/checkout@v2
+      with:
+        repository: 'github/codeql-go'
+        path: codeql-go
+    - name: Set up Python 3.8
+      uses: actions/setup-python@v2
+      with:
+        python-version: 3.8
+    - name: Download CodeQL CLI
+      uses: dsaltares/fetch-gh-release-asset@aa37ae5c44d3c9820bc12fe675e8670ecd93bd1c
+      with:
+        repo: "github/codeql-cli-binaries"
+        version: "latest"
+        file: "codeql-linux64.zip"
+        token: ${{ secrets.GITHUB_TOKEN }}
+    - name: Unzip CodeQL CLI
+      run: unzip -d codeql-cli codeql-linux64.zip
+    - name: Set up query help docs folder  
+      run: |
+        cp -r codeql/docs/codeql/** .
+    - name: Query help to markdown
+      run: |
+        PATH="$PATH:codeql-cli/codeql" python codeql/docs/codeql/query-help-markdown.py
+    - name: Run Sphinx for query help
+      uses: ammaraskar/sphinx-action@8b4f60114d7fd1faeba1a712269168508d4750d2 # v0.4
+      with:
+        docs-folder: "query-help/"
+        pre-build-command: "python -m pip install --upgrade recommonmark"
+        build-command: "sphinx-build -b dirhtml . _build" 
+    - name: Upload HTML artifacts
+      uses: actions/upload-artifact@v2
+      with:
+        name: query-help-html
+        path: query-help/_build
+    

.gitignore

@@ -17,6 +17,9 @@
 # Byte-compiled python files
 *.pyc
 
+# python virtual environment folder 
+.venv/
+
 # It's useful (though not required) to be able to unpack codeql in the ql checkout itself
 /codeql/
 

CONTRIBUTING.md

@@ -38,6 +38,8 @@ 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.
+
 4. **Compilation**
 
     - Compilation of the query and any associated libraries and tests must be resilient to future development of the [supported](docs/supported-queries.md) libraries. This means that the functionality cannot use internal libraries, cannot depend on the output of `getAQlClass`, and cannot make use of regexp matching on `toString`.

change-notes/1.26/analysis-python.md

@@ -4,19 +4,34 @@ The following changes in version 1.26 affect Python analysis in all applications
 
 ## General improvements
 
-
-## New queries
-
-| **Query**                   | **Tags**  | **Purpose**                                                        |
-|-----------------------------|-----------|--------------------------------------------------------------------|
-
-
 ## Changes to existing queries
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
-
-
+|`py/unsafe-deserialization` | Different results. | The underlying data flow library has been changed. See below for more details. |
+|`py/path-injection` | Different results. | The underlying data flow library has been changed. See below for more details. |
+|`py/command-line-injection` | Different results. | The underlying data flow library has been changed. See below for more details. |
+|`py/reflective-xss` | Different results. | The underlying data flow library has been changed. See below for more details. |
+|`py/sql-injection` | Different results. | The underlying data flow library has been changed. See below for more details. |
+|`py/code-injection` | Different results. | The underlying data flow library has been changed. See below for more details. |
 ## Changes to libraries
-
+* Some of the security queries now use the shared data flow library for data flow and taint tracking. This has resulted in an overall more robust and accurate analysis. The libraries mentioned below have been modelled in this new framework. Other libraries (e.g. the web framework `CherryPy`) have not been modelled yet, and this may lead to a temporary loss of results for these frameworks.
+* Improved modelling of the following serialization libraries:
+  - `PyYAML`
+  - `dill`
+  - `pickle`
+  - `marshal`
+* Improved modelling of the following web frameworks:
+  - `Django` (Note that modelling of class-based response handlers is currently incomplete.)
+  - `Flask`
+* Support for Werkzeug `MultiDict`.
+* Support for the [Python Database API Specification v2.0 (PEP-249)](https://www.python.org/dev/peps/pep-0249/), including the following libraries:
+  - `MySQLdb`
+  - `mysql-connector-python`
+  - `django.db`
+* Improved modelling of the following command execution libraries:
+  - `Fabric`
+  - `Invoke`
+* Improved modelling of security-related standard library modules, such as `os`, `popen2`, `platform`, and `base64`.
+* The original versions of the updated queries have been preserved [here](https://github.com/github/codeql/tree/main/python/ql/src/experimental/Security-old-dataflow).
 * Added taint tracking support for string formatting through f-strings.

config/identical-files.json

@@ -358,6 +358,14 @@
     "cpp/ql/test/TestUtilities/InlineExpectationsTest.qll",
     "python/ql/test/TestUtilities/InlineExpectationsTest.qll"
   ],
+  "C++ ExternalAPIs": [
+    "cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll",
+    "cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll"
+  ],
+  "C++ SafeExternalAPIFunction": [
+    "cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll",
+    "cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll"
+  ],
   "XML": [
     "cpp/ql/src/semmle/code/cpp/XML.qll",
     "csharp/ql/src/semmle/code/csharp/XML.qll",

cpp/change-notes/2020-02-04-unsigned-difference-expression-compared-zero.md

@@ -0,0 +1,2 @@
+lgtm
+* A new query (`cpp/unsigned-difference-expression-compared-zero`) is run but not yet displayed on LGTM. The query finds unsigned subtractions used in relational comparisons with the value 0. This query was originally submitted as an experimental query by @ihsinme in https://github.com/github/codeql/pull/4745.

cpp/change-notes/2020-11-05-formatting-function.md

@@ -0,0 +1,4 @@
+lgtm,codescanning
+* `FormattingFunction.getOutputParameterIndex` now has a parameter identifying whether the output at that index is a buffer or a stream.
+* `FormattingFunction` now has a predicate `isOutputGlobal` indicating when the output is to a global stream.
+* The `primitiveVariadicFormatter` and `variadicFormatter` predicates have more parameters exposing information about the function.

cpp/change-notes/2020-11-05-private-models.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* Various classes in `semmle.code.cpp.models.implementations` have been made private. Users should not depend on library implementation details.
+* The `OperatorNewAllocationFunction`, `OperatorDeleteDeallocationFunction`, `Iterator` and `Snprintf` classes now have interfaces in `semmle.code.cpp.models.interfaces`.

cpp/change-notes/2020-11-12-unsafe-use-of-this.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query (`cpp/unsafe-use-of-this`) has been added. The query finds pure virtual function calls whose qualifier is an object under construction.

cpp/change-notes/2020-11-27-downgrade-to-recommendation.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The queries `cpp/local-variable-hides-global-variable` and `cpp/missing-header-guard` now have severity `recommendation` instead of `warning`.

cpp/config/suites/cpp/correctness

@@ -9,6 +9,8 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
++ semmlecode-cpp-queries/Likely Bugs/OO/UnsafeUseOfThis.ql: /Correctness/Dangerous Conversions
++ semmlecode-cpp-queries/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.ql

@@ -4,9 +4,6 @@
  * @kind graph
  * @id cpp/architecture/class-hierarchies
  * @graph.layout organic
- * @workingset jhotdraw
- * @result succeed 48
- * @result_ondemand succeed 48
  * @tags maintainability
  */
 

cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.ql

@@ -4,9 +4,6 @@
  * @kind chart
  * @id cpp/architecture/inheritance-depth-distribution
  * @chart.type line
- * @workingset jhotdraw
- * @result succeed 48
- * @result_ondemand succeed 48
  * @tags maintainability
  */
 

cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.ql

@@ -1,7 +1,8 @@
 /**
  * @name Global namespace classes
  * @description Finds classes that belong to no namespace.
- * @kind table
+ * @kind problem
+ * @problem.severity recommendation
  * @id cpp/architecture/global-namespace-classes
  * @tags maintainability
  *       modularity

cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyDependencies.ql

@@ -4,9 +4,6 @@
  * @kind problem
  * @id cpp/architecture/classes-with-many-dependencies
  * @problem.severity recommendation
- * @workingset jhotdraw
- * @result succeed 20
- * @result_ondemand succeed 20
  * @tags maintainability
  *       statistical
  *       non-attributable

cpp/ql/src/Best Practices/Hiding/LocalVariableHidesGlobalVariable.ql

@@ -2,7 +2,7 @@
  * @name Local variable hides global variable
  * @description A local variable or parameter that hides a global variable of the same name. This may be confusing. Consider renaming one of the variables.
  * @kind problem
- * @problem.severity warning
+ * @problem.severity recommendation
  * @precision very-high
  * @id cpp/local-variable-hides-global-variable
  * @tags maintainability

cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll

@@ -8,168 +8,41 @@ import semmle.code.cpp.AutogeneratedFile
 predicate trivialPositiveIntValue(string s) {
   // Small numbers
   s = [0 .. 20].toString() or
-  // Popular powers of two (decimal)
-  s = "16" or
-  s = "24" or
-  s = "32" or
-  s = "64" or
-  s = "128" or
-  s = "256" or
-  s = "512" or
-  s = "1024" or
-  s = "2048" or
-  s = "4096" or
-  s = "16384" or
-  s = "32768" or
-  s = "65536" or
-  s = "1048576" or
-  s = "2147483648" or
-  s = "4294967296" or
-  // Popular powers of two, minus one (decimal)
-  s = "15" or
-  s = "31" or
-  s = "63" or
-  s = "127" or
-  s = "255" or
-  s = "511" or
-  s = "1023" or
-  s = "2047" or
-  s = "4095" or
-  s = "16383" or
-  s = "32767" or
-  s = "65535" or
-  s = "1048577" or
-  s = "2147483647" or
-  s = "4294967295" or
-  // Popular powers of two (32-bit hex)
-  s = "0x00000001" or
-  s = "0x00000002" or
-  s = "0x00000004" or
-  s = "0x00000008" or
-  s = "0x00000010" or
-  s = "0x00000020" or
-  s = "0x00000040" or
-  s = "0x00000080" or
-  s = "0x00000100" or
-  s = "0x00000200" or
-  s = "0x00000400" or
-  s = "0x00000800" or
-  s = "0x00001000" or
-  s = "0x00002000" or
-  s = "0x00004000" or
-  s = "0x00008000" or
-  s = "0x00010000" or
-  s = "0x00020000" or
-  s = "0x00040000" or
-  s = "0x00080000" or
-  s = "0x00100000" or
-  s = "0x00200000" or
-  s = "0x00400000" or
-  s = "0x00800000" or
-  s = "0x01000000" or
-  s = "0x02000000" or
-  s = "0x04000000" or
-  s = "0x08000000" or
-  s = "0x10000000" or
-  s = "0x20000000" or
-  s = "0x40000000" or
-  s = "0x80000000" or
-  // Popular powers of two, minus one (32-bit hex)
-  s = "0x00000001" or
-  s = "0x00000003" or
-  s = "0x00000007" or
-  s = "0x0000000f" or
-  s = "0x0000001f" or
-  s = "0x0000003f" or
-  s = "0x0000007f" or
-  s = "0x000000ff" or
-  s = "0x000001ff" or
-  s = "0x000003ff" or
-  s = "0x000007ff" or
-  s = "0x00000fff" or
-  s = "0x00001fff" or
-  s = "0x00003fff" or
-  s = "0x00007fff" or
-  s = "0x0000ffff" or
-  s = "0x0001ffff" or
-  s = "0x0003ffff" or
-  s = "0x0007ffff" or
-  s = "0x000fffff" or
-  s = "0x001fffff" or
-  s = "0x003fffff" or
-  s = "0x007fffff" or
-  s = "0x00ffffff" or
-  s = "0x01ffffff" or
-  s = "0x03ffffff" or
-  s = "0x07ffffff" or
-  s = "0x0fffffff" or
-  s = "0x1fffffff" or
-  s = "0x3fffffff" or
-  s = "0x7fffffff" or
-  s = "0xffffffff" or
-  // Popular powers of two (16-bit hex)
-  s = "0x0001" or
-  s = "0x0002" or
-  s = "0x0004" or
-  s = "0x0008" or
-  s = "0x0010" or
-  s = "0x0020" or
-  s = "0x0040" or
-  s = "0x0080" or
-  s = "0x0100" or
-  s = "0x0200" or
-  s = "0x0400" or
-  s = "0x0800" or
-  s = "0x1000" or
-  s = "0x2000" or
-  s = "0x4000" or
-  s = "0x8000" or
-  // Popular powers of two, minus one (16-bit hex)
-  s = "0x0001" or
-  s = "0x0003" or
-  s = "0x0007" or
-  s = "0x000f" or
-  s = "0x001f" or
-  s = "0x003f" or
-  s = "0x007f" or
-  s = "0x00ff" or
-  s = "0x01ff" or
-  s = "0x03ff" or
-  s = "0x07ff" or
-  s = "0x0fff" or
-  s = "0x1fff" or
-  s = "0x3fff" or
-  s = "0x7fff" or
-  s = "0xffff" or
-  // Popular powers of two (8-bit hex)
-  s = "0x01" or
-  s = "0x02" or
-  s = "0x04" or
-  s = "0x08" or
-  s = "0x10" or
-  s = "0x20" or
-  s = "0x40" or
-  s = "0x80" or
-  // Popular powers of two, minus one (8-bit hex)
-  s = "0x01" or
-  s = "0x03" or
-  s = "0x07" or
-  s = "0x0f" or
-  s = "0x1f" or
-  s = "0x3f" or
-  s = "0x7f" or
-  s = "0xff" or
-  s = "0x00" or
-  // Powers of ten
-  s = "10" or
-  s = "100" or
-  s = "1000" or
-  s = "10000" or
-  s = "100000" or
-  s = "1000000" or
-  s = "10000000" or
-  s = "100000000" or
-  s = "1000000000"
+  s =
+    [
+      // Popular powers of two (decimal)
+      "16", "24", "32", "64", "128", "256", "512", "1024", "2048", "4096", "16384", "32768",
+      "65536", "1048576", "2147483648", "4294967296",
+      // Popular powers of two, minus one (decimal)
+      "15", "31", "63", "127", "255", "511", "1023", "2047", "4095", "16383", "32767", "65535",
+      "1048577", "2147483647", "4294967295",
+      // Popular powers of two (32-bit hex)
+      "0x00000001", "0x00000002", "0x00000004", "0x00000008", "0x00000010", "0x00000020",
+      "0x00000040", "0x00000080", "0x00000100", "0x00000200", "0x00000400", "0x00000800",
+      "0x00001000", "0x00002000", "0x00004000", "0x00008000", "0x00010000", "0x00020000",
+      "0x00040000", "0x00080000", "0x00100000", "0x00200000", "0x00400000", "0x00800000",
+      "0x01000000", "0x02000000", "0x04000000", "0x08000000", "0x10000000", "0x20000000",
+      "0x40000000", "0x80000000",
+      // Popular powers of two, minus one (32-bit hex)
+      "0x00000001", "0x00000003", "0x00000007", "0x0000000f", "0x0000001f", "0x0000003f",
+      "0x0000007f", "0x000000ff", "0x000001ff", "0x000003ff", "0x000007ff", "0x00000fff",
+      "0x00001fff", "0x00003fff", "0x00007fff", "0x0000ffff", "0x0001ffff", "0x0003ffff",
+      "0x0007ffff", "0x000fffff", "0x001fffff", "0x003fffff", "0x007fffff", "0x00ffffff",
+      "0x01ffffff", "0x03ffffff", "0x07ffffff", "0x0fffffff", "0x1fffffff", "0x3fffffff",
+      "0x7fffffff", "0xffffffff",
+      // Popular powers of two (16-bit hex)
+      "0x0001", "0x0002", "0x0004", "0x0008", "0x0010", "0x0020", "0x0040", "0x0080", "0x0100",
+      "0x0200", "0x0400", "0x0800", "0x1000", "0x2000", "0x4000", "0x8000",
+      // Popular powers of two, minus one (16-bit hex)
+      "0x0001", "0x0003", "0x0007", "0x000f", "0x001f", "0x003f", "0x007f", "0x00ff", "0x01ff",
+      "0x03ff", "0x07ff", "0x0fff", "0x1fff", "0x3fff", "0x7fff", "0xffff",
+      // Popular powers of two (8-bit hex)
+      "0x01", "0x02", "0x04", "0x08", "0x10", "0x20", "0x40", "0x80",
+      // Popular powers of two, minus one (8-bit hex)
+      "0x01", "0x03", "0x07", "0x0f", "0x1f", "0x3f", "0x7f", "0xff", "0x00",
+      // Powers of ten
+      "10", "100", "1000", "10000", "100000", "1000000", "10000000", "100000000", "1000000000"
+    ]
 }
 
 predicate trivialIntValue(string s) {
@@ -235,10 +108,7 @@ predicate joiningStringTrivial(Literal lit) {
   // understand (which is against the spirit of these queries).
   stringLiteral(lit) and
   exists(FunctionCall fc |
-    (
-      fc.getTarget().getName() = "operator+" or
-      fc.getTarget().getName() = "operator<<"
-    ) and
+    fc.getTarget().getName() = ["operator+", "operator<<"] and
     fc.getAnArgument().getAChild*() = lit
   ) and
   lit.getValue().length() < 16
@@ -291,8 +161,7 @@ predicate arrayInitializerChild(AggregateLiteral parent, Expr e) {
 
 // i.e. not a constant folded expression
 predicate literallyLiteral(Literal lit) {
-  lit
-      .getValueText()
+  lit.getValueText()
       .regexpMatch(".*\".*|\\s*+[-+]?+\\s*+(0[xob][0-9a-fA-F]|[0-9])[0-9a-fA-F,._]*+([eE][-+]?+[0-9,._]*+)?+\\s*+[a-zA-Z]*+\\s*+")
 }
 

cpp/ql/src/Critical/NewDelete.qll

@@ -5,8 +5,6 @@
 import cpp
 import semmle.code.cpp.controlflow.SSA
 import semmle.code.cpp.dataflow.DataFlow
-import semmle.code.cpp.models.implementations.Allocation
-import semmle.code.cpp.models.implementations.Deallocation
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is

cpp/ql/src/DefaultOptions.qll

@@ -59,14 +59,9 @@ class Options extends string {
   predicate exits(Function f) {
     f.getAnAttribute().hasName("noreturn")
     or
-    exists(string name | f.hasGlobalOrStdName(name) |
-      name = "exit" or
-      name = "_exit" or
-      name = "abort" or
-      name = "__assert_fail" or
-      name = "longjmp" or
-      name = "__builtin_unreachable"
-    )
+    f.hasGlobalOrStdName([
+        "exit", "_exit", "abort", "__assert_fail", "longjmp", "__builtin_unreachable"
+      ])
     or
     CustomOptions::exits(f) // old Options.qll
   }

cpp/ql/src/JPL_C/LOC-2/Rule 05/HeapMemory.ql

@@ -21,15 +21,7 @@ class Initialization extends Function {
 }
 
 class Allocation extends FunctionCall {
-  Allocation() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "malloc" or
-      name = "calloc" or
-      name = "alloca" or
-      name = "sbrk" or
-      name = "valloc"
-    )
-  }
+  Allocation() { this.getTarget().getName() = ["malloc", "calloc", "alloca", "sbrk", "valloc"] }
 }
 
 from Function f, Allocation a

cpp/ql/src/JPL_C/LOC-2/Rule 07/ThreadSafety.ql

@@ -13,13 +13,8 @@ import cpp
 
 class ForbiddenCall extends FunctionCall {
   ForbiddenCall() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "task_delay" or
-      name = "taskDelay" or
-      name = "sleep" or
-      name = "nanosleep" or
-      name = "clock_nanosleep"
-    )
+    this.getTarget().getName() =
+      ["task_delay", "taskDelay", "sleep", "nanosleep", "clock_nanosleep"]
   }
 }
 

cpp/ql/src/JPL_C/LOC-2/Rule 09/Semaphores.qll

@@ -6,12 +6,7 @@ import cpp
 
 class SemaphoreCreation extends FunctionCall {
   SemaphoreCreation() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "semBCreate" or
-      name = "semMCreate" or
-      name = "semCCreate" or
-      name = "semRWCreate"
-    )
+    this.getTarget().getName() = ["semBCreate", "semMCreate", "semCCreate", "semRWCreate"]
   }
 
   Variable getSemaphore() { result.getAnAccess() = this.getParent().(Assignment).getLValue() }
@@ -72,11 +67,7 @@ class SemaphoreGive extends UnlockOperation {
 }
 
 class LockingPrimitive extends FunctionCall, LockOperation {
-  LockingPrimitive() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "taskLock" or name = "intLock" or name = "taskRtpLock"
-    )
-  }
+  LockingPrimitive() { this.getTarget().getName() = ["taskLock", "intLock", "taskRtpLock"] }
 
   override Function getLocked() { result = this.getTarget() }
 
@@ -89,11 +80,7 @@ class LockingPrimitive extends FunctionCall, LockOperation {
 }
 
 class UnlockingPrimitive extends FunctionCall, UnlockOperation {
-  UnlockingPrimitive() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "taskUnlock" or name = "intUnlock" or name = "taskRtpUnlock"
-    )
-  }
+  UnlockingPrimitive() { this.getTarget().getName() = ["taskUnlock", "intUnlock", "taskRtpUnlock"] }
 
   Function getLocked() { result = getMatchingLock().getLocked() }
 

cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql

@@ -12,18 +12,7 @@
 import cpp
 
 predicate allowedTypedefs(TypedefType t) {
-  exists(string name | name = t.getName() |
-    name = "I64" or
-    name = "U64" or
-    name = "I32" or
-    name = "U32" or
-    name = "I16" or
-    name = "U16" or
-    name = "I8" or
-    name = "U8" or
-    name = "F64" or
-    name = "F32"
-  )
+  t.getName() = ["I64", "U64", "I32", "U32", "I16", "U16", "I8", "U8", "F64", "F32"]
 }
 
 /**

cpp/ql/src/JPL_C/Tasks.qll

@@ -5,8 +5,8 @@ import cpp
  */
 class Task extends Function {
   Task() {
-    exists(FunctionCall taskCreate, string name | name = "taskCreate" or name = "taskSpawn" |
-      name = taskCreate.getTarget().getName() and
+    exists(FunctionCall taskCreate |
+      taskCreate.getTarget().getName() = ["taskCreate", "taskSpawn"] and
       this = taskCreate.getArgument(4).(AddressOfExpr).getAddressable()
     )
   }

cpp/ql/src/Likely Bugs/Conversion/LossyFunctionResultCast.ql

@@ -13,38 +13,17 @@ import cpp
 import semmle.code.cpp.dataflow.DataFlow
 
 predicate whitelist(Function f) {
-  exists(string fName |
-    fName = f.getName() and
-    (
-      fName = "ceil" or
-      fName = "ceilf" or
-      fName = "ceill" or
-      fName = "floor" or
-      fName = "floorf" or
-      fName = "floorl" or
-      fName = "nearbyint" or
-      fName = "nearbyintf" or
-      fName = "nearbyintl" or
-      fName = "rint" or
-      fName = "rintf" or
-      fName = "rintl" or
-      fName = "round" or
-      fName = "roundf" or
-      fName = "roundl" or
-      fName = "trunc" or
-      fName = "truncf" or
-      fName = "truncl" or
-      fName.matches("__builtin_%")
-    )
-  )
+  f.getName() =
+    [
+      "ceil", "ceilf", "ceill", "floor", "floorf", "floorl", "nearbyint", "nearbyintf",
+      "nearbyintl", "rint", "rintf", "rintl", "round", "roundf", "roundl", "trunc", "truncf",
+      "truncl"
+    ] or
+  f.getName().matches("__builtin_%")
 }
 
 predicate whitelistPow(FunctionCall fc) {
-  (
-    fc.getTarget().getName() = "pow" or
-    fc.getTarget().getName() = "powf" or
-    fc.getTarget().getName() = "powl"
-  ) and
+  fc.getTarget().getName() = ["pow", "powf", "powl"] and
   exists(float value |
     value = fc.getArgument(0).getValue().toFloat() and
     (value.floor() - value).abs() < 0.001

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.cpp

@@ -0,0 +1,20 @@
+class Base {
+private:
+    // pure virtual member function used for initialization of derived classes.
+    virtual void construct() = 0;
+public:
+    Base() {
+        // wrong: the virtual table of `Derived` has not been initialized yet. So this
+        // call will resolve to `Base::construct`, which cannot be called as it is a pure
+        // virtual function.
+        construct();
+    }
+};
+
+class Derived : public Base {
+    int field;
+
+    void construct() override {
+        field = 1;
+    }
+};

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.qhelp

@@ -0,0 +1,30 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>This rule finds calls to pure virtual member functions in constructors and destructors. When executing the body of a constructor of class <code>T</code>, the virtual table of <code>T</code> refers to the virtual table of one of <code>T</code>'s base classes. This can produce unexpected behavior, including program abort that can lead to denial of service attacks. The same problem exists during destruction of an object.</p>
+
+</overview>
+<recommendation>
+<p>Do not rely on virtual dispatch in constructors and destructors. Instead, each class should be responsible for acquiring and releasing its resources. If a base class needs to refer to a derived class during initialization, use the Dynamic Binding During Initialization idiom.</p>
+
+</recommendation>
+<example><sample src="UnsafeUseOfThis.cpp" />
+
+
+
+</example>
+<references>
+
+<li>ISO C++ FAQ: <a href="https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctors">When my base class's constructor calls a virtual function on its this object, why doesn't my derived class's override of that virtual function get invoked?</a>
+</li>
+<li>SEI CERT C++ Coding Standard <a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP50-CPP.+Do+not+invoke+virtual+functions+from+constructors+or+destructors">OOP50-CPP. Do not invoke virtual functions from constructors or destructors</a>
+</li>
+<li>ISO C++ FAQ: <a href="https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctor-idiom">Okay, but is there a way to simulate that behavior as if dynamic binding worked on the this object within my base class's constructor?</a>
+</li>
+
+
+</references></qhelp>

cpp/ql/src/Likely Bugs/OO/UnsafeUseOfThis.ql

@@ -0,0 +1,212 @@
+/**
+ * @name Unsafe use of this in constructor
+ * @description A call to a pure virtual function using a 'this'
+ *              pointer of an object that is under construction
+ *              may lead to undefined behavior.
+ * @kind path-problem
+ * @id cpp/unsafe-use-of-this
+ * @problem.severity error
+ * @precision very-high
+ * @tags correctness
+ *       language-features
+ *       security
+ */
+
+import cpp
+// We don't actually use the global value numbering library in this query, but without it we end up
+// recomputing the IR.
+private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+private import semmle.code.cpp.ir.IR
+
+bindingset[n, result]
+int unbind(int n) { result >= n and result <= n }
+
+/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
+predicate parameterOf(Parameter p, Function f, int n) {
+  not f.isVirtual() and f.getParameter(n) = p
+}
+
+/**
+ * Holds if `instr` is the `n`'th argument to a call to the non-virtual function `f`, and
+ * `init` is the corresponding initiazation instruction that receives the value of `instr` in `f`.
+ */
+predicate flowIntoParameter(
+  CallInstruction call, Instruction instr, Function f, int n, InitializeParameterInstruction init
+) {
+  not f.isVirtual() and
+  call.getPositionalArgument(n) = instr and
+  f = call.getStaticCallTarget() and
+  getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
+  init.getParameter().getIndex() = unbind(n)
+}
+
+/**
+ * Holds if `instr` is an argument to a call to the function `f`, and `init` is the
+ * corresponding initialization instruction that receives the value of `instr` in `f`.
+ */
+pragma[noinline]
+predicate getPositionalArgumentInitParam(
+  CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
+) {
+  exists(int n |
+    parameterOf(_, f, n) and
+    flowIntoParameter(call, instr, f, unbind(n), init)
+  )
+}
+
+/**
+ * Holds if `instr` is the qualifier to a call to the non-virtual function `f`, and
+ * `init` is the corresponding initiazation instruction that receives the value of
+ * `instr` in `f`.
+ */
+pragma[noinline]
+predicate getThisArgumentInitParam(
+  CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
+) {
+  not f.isVirtual() and
+  call.getStaticCallTarget() = f and
+  getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
+  call.getThisArgument() = instr and
+  init.getIRVariable() instanceof IRThisVariable
+}
+
+/** Holds if `instr` is a `this` pointer used by the call instruction `call`. */
+predicate isSink(Instruction instr, CallInstruction call) {
+  exists(PureVirtualFunction func |
+    call.getStaticCallTarget() = func and
+    call.getThisArgument() = instr and
+    // Weed out implicit calls to destructors of a base class
+    not func instanceof Destructor
+  )
+}
+
+/** Holds if `init` initializes the `this` pointer in class `c`. */
+predicate isSource(InitializeParameterInstruction init, string msg, Class c) {
+  (
+    exists(Constructor func |
+      not func instanceof CopyConstructor and
+      not func instanceof MoveConstructor and
+      func = init.getEnclosingFunction() and
+      msg = "construction"
+    )
+    or
+    init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
+  ) and
+  init.getIRVariable() instanceof IRThisVariable and
+  init.getEnclosingFunction().getDeclaringType() = c
+}
+
+/**
+ * Holds if `instr` flows to a sink (which is a use of the value of `instr` as a `this` pointer).
+ */
+predicate flowsToSink(Instruction instr, Instruction sink) {
+  flowsFromSource(instr) and
+  (
+    isSink(instr, _) and instr = sink
+    or
+    exists(Instruction mid |
+      successor(instr, mid) and
+      flowsToSink(mid, sink)
+    )
+  )
+}
+
+/** Holds if `instr` flows from a source. */
+predicate flowsFromSource(Instruction instr) {
+  isSource(instr, _, _)
+  or
+  exists(Instruction mid |
+    successor(mid, instr) and
+    flowsFromSource(mid)
+  )
+}
+
+/** Holds if `f` is the enclosing non-virtual function of `init`. */
+predicate getEnclosingNonVirtualFunctionInitializeParameter(
+  InitializeParameterInstruction init, Function f
+) {
+  not f.isVirtual() and
+  init.getEnclosingFunction() = f
+}
+
+/** Holds if `f` is the enclosing non-virtual function of `init`. */
+predicate getEnclosingNonVirtualFunctionInitializeIndirection(
+  InitializeIndirectionInstruction init, Function f
+) {
+  not f.isVirtual() and
+  init.getEnclosingFunction() = f
+}
+
+/**
+ * Holds if `instr` is an argument (or argument indirection) to a call, and
+ * `succ` is the corresponding initialization instruction in the call target.
+ */
+predicate flowThroughCallable(Instruction instr, Instruction succ) {
+  // Flow from an argument to a parameter
+  exists(CallInstruction call, InitializeParameterInstruction init | init = succ |
+    getPositionalArgumentInitParam(call, instr, init, call.getStaticCallTarget())
+    or
+    getThisArgumentInitParam(call, instr, init, call.getStaticCallTarget())
+  )
+  or
+  // Flow from argument indirection to parameter indirection
+  exists(
+    CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
+  |
+    init = succ and
+    read.getPrimaryInstruction() = call and
+    getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
+  |
+    exists(int n |
+      read.getSideEffectOperand().getAnyDef() = instr and
+      read.getIndex() = n and
+      init.getParameter().getIndex() = unbind(n)
+    )
+    or
+    call.getThisArgument() = instr and
+    init.getIRVariable() instanceof IRThisVariable
+  )
+}
+
+/** Holds if `instr` flows to `succ`. */
+predicate successor(Instruction instr, Instruction succ) {
+  succ.(CopyInstruction).getSourceValue() = instr or
+  succ.(CheckedConvertOrNullInstruction).getUnary() = instr or
+  succ.(ChiInstruction).getTotal() = instr or
+  succ.(ConvertInstruction).getUnary() = instr or
+  succ.(InheritanceConversionInstruction).getUnary() = instr or
+  flowThroughCallable(instr, succ)
+}
+
+/**
+ * Holds if:
+ * - `source` is an initialization of a `this` pointer of type `sourceClass`, and
+ * - `sink` is a use of the `this` pointer, and
+ * - `call` invokes a pure virtual function using `sink` as the `this` pointer, and
+ * - `msg` is a string describing whether `source` is from a constructor or destructor.
+ */
+predicate flows(
+  Instruction source, string msg, Class sourceClass, Instruction sink, CallInstruction call
+) {
+  isSource(source, msg, sourceClass) and
+  flowsToSink(source, sink) and
+  isSink(sink, call)
+}
+
+query predicate edges(Instruction a, Instruction b) { successor(a, b) and flowsToSink(b, _) }
+
+query predicate nodes(Instruction n, string key, string val) {
+  flowsToSink(n, _) and
+  key = "semmle.label" and
+  val = n.toString()
+}
+
+from Instruction source, Instruction sink, CallInstruction call, string msg, Class sourceClass
+where
+  flows(source, msg, sourceClass, sink, call) and
+  // Only raise an alert if there is no override of the pure virtual function in any base class.
+  not exists(Class c | c = sourceClass.getABaseClass*() |
+    c.getAMemberFunction().getAnOverriddenFunction() = call.getStaticCallTarget()
+  )
+select call.getUnconvertedResultExpression(), source, sink,
+  "Call to pure virtual function during " + msg

cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.qhelp

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
+unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
+may be relevant for security analysis of this application.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not
+modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
+third party dependencies or from internal dependencies. The query will report the function name, along with
+either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
+untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
+function call.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, no action is required.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
+  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>If the query were to return the API <code>fputs [param 1]</code> 
+then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
+consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
+
+<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
+reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-020/CountUntrustedDataToExternalAPI.ql

@@ -0,0 +1,17 @@
+/**
+ * @name Frequency counts for external APIs that are used with untrusted data
+ * @description This reports the external APIs that are used with untrusted data, along with how
+ *              frequently the API is called, and how many unique sources of untrusted data flow
+ *              to it.
+ * @id cpp/count-untrusted-data-external-api
+ * @kind table
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import ExternalAPIs
+
+from ExternalAPIUsedWithUntrustedData externalAPI
+select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
+  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
+    numberOfUntrustedSources desc

cpp/ql/src/Security/CWE/CWE-020/ExternalAPISinkExample.cpp

@@ -0,0 +1,13 @@
+#include <cstdio>
+
+void do_get(FILE* request, FILE* response) {
+  char page[1024];
+  fgets(page, 1024, request);
+
+  char buffer[1024];
+  strcat(buffer, "The page \"");
+  strcat(buffer, page);
+  strcat(buffer, "\" was not found.");
+
+  fputs(buffer, response);
+}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPITaintStepExample.cpp

@@ -0,0 +1,13 @@
+#include <cstdio>
+
+void do_get(FILE* request, FILE* response) {
+  char user_id[1024];
+  fgets(user_id, 1024, request);
+
+  char buffer[1024];
+  strcat(buffer, "SELECT * FROM user WHERE user_id='");
+  strcat(buffer, user_id);
+  strcat(buffer, "'");
+
+  // ...
+}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll

@@ -0,0 +1,50 @@
+/**
+ * Definitions for reasoning about untrusted data used in APIs defined outside the
+ * database.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.Taint
+import ExternalAPIsSpecific
+
+/** A node representing untrusted data being passed to an external API. */
+class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
+  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
+
+  /** Gets a source of untrusted data which is passed to this external API data node. */
+  DataFlow::Node getAnUntrustedSource() {
+    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
+  }
+}
+
+private newtype TExternalAPI =
+  TExternalAPIParameter(Function f, int index) {
+    exists(UntrustedExternalAPIDataNode n |
+      f = n.getExternalFunction() and
+      index = n.getIndex()
+    )
+  }
+
+/** An external API which is used with untrusted data. */
+class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
+  /** Gets a possibly untrusted use of this external API. */
+  UntrustedExternalAPIDataNode getUntrustedDataNode() {
+    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
+  }
+
+  /** Gets the number of untrusted sources used with this external API. */
+  int getNumberOfUntrustedSources() {
+    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
+  }
+
+  /** Gets a textual representation of this element. */
+  string toString() {
+    exists(Function f, int index, string indexString |
+      if index = -1 then indexString = "qualifier" else indexString = "param " + index
+    |
+      this = TExternalAPIParameter(f, index) and
+      result = f.toString() + " [" + indexString + "]"
+    )
+  }
+}

cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll

@@ -0,0 +1,56 @@
+/**
+ * Provides AST-specific definitions for use in the `ExternalAPI` library.
+ */
+
+import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.models.interfaces.FlowSource
+import semmle.code.cpp.models.interfaces.DataFlow
+import SafeExternalAPIFunction
+
+/** A node representing untrusted data being passed to an external API. */
+class ExternalAPIDataNode extends DataFlow::Node {
+  Call call;
+  int i;
+
+  ExternalAPIDataNode() {
+    // Argument to call to a function
+    (
+      this.asExpr() = call.getArgument(i)
+      or
+      i = -1 and this.asExpr() = call.getQualifier()
+    ) and
+    exists(Function f |
+      f = call.getTarget() and
+      // Defined outside the source archive
+      not f.hasDefinition() and
+      // Not already modeled as a dataflow or taint step
+      not f instanceof DataFlowFunction and
+      not f instanceof TaintFunction and
+      // Not a call to a known safe external API
+      not f instanceof SafeExternalAPIFunction
+    )
+  }
+
+  /** Gets the called API `Function`. */
+  Function getExternalFunction() { result = call.getTarget() }
+
+  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
+  int getIndex() { result = i }
+
+  /** Gets the description of the function being called. */
+  string getFunctionDescription() { result = getExternalFunction().toString() }
+}
+
+/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
+class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(RemoteFlowFunction remoteFlow |
+      remoteFlow = source.asExpr().(Call).getTarget() and
+      remoteFlow.hasRemoteFlowSource(_, _)
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
+}

cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.qhelp

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+all external APIs that are used with untrusted data, along with how frequently the API is used, and how many 
+unique sources of untrusted data flow to this API. This query is designed primarily to help identify which APIs 
+may be relevant for security analysis of this application.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not
+modeled as a taint step in the default taint library. External APIs may be from the C++ standard library,
+third party dependencies or from internal dependencies. The query will report the function name, along with
+either <code>[param x]</code>, where <code>x</code> indicates the position of the parameter receiving the
+untrusted data or <code>[qualifier]</code> indicating the untrusted data is used as the qualifier to the
+function call.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, no action is required.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query.</li>
+  <li>If the result represents a call to an external API which transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>If the query were to return the API <code>fputs [param 1]</code> 
+then we should first consider whether this a security relevant sink. In this case, this is writing to a <code>FILE*</code>, so we should 
+consider whether this is an XSS sink. If it is, we should confirm that it is handled by the XSS query.</p>
+
+<p>If the query were to return the API <code>strcat [param 1]</code>, then this should be
+reviewed as a possible taint step, because tainted data would flow from the 1st argument to the 0th argument of the call.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-020/IRCountUntrustedDataToExternalAPI.ql

@@ -0,0 +1,17 @@
+/**
+ * @name Frequency counts for external APIs that are used with untrusted data
+ * @description This reports the external APIs that are used with untrusted data, along with how
+ *              frequently the API is called, and how many unique sources of untrusted data flow
+ *              to it.
+ * @id cpp/count-untrusted-data-external-api-ir
+ * @kind table
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import ir.ExternalAPIs
+
+from ExternalAPIUsedWithUntrustedData externalAPI
+select externalAPI, count(externalAPI.getUntrustedDataNode()) as numberOfUses,
+  externalAPI.getNumberOfUntrustedSources() as numberOfUntrustedSources order by
+    numberOfUntrustedSources desc

cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.qhelp

@@ -0,0 +1,59 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
+The query provides data for security reviews of the application and you can also use it to identify external APIs
+that should be modeled as either taint steps, or sinks for specific problems.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
+as a taint step in the default taint library. External APIs may be from the
+C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
+untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
+  that the result is a false positive because this data is sanitized.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
+  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
+  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
+<code>fputs</code> external API:</p>
+
+<sample src="ExternalAPISinkExample.cpp" />
+
+<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
+and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
+some existing sanitization.</p>
+
+<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
+
+<sample src="ExternalAPITaintStepExample.cpp" />
+
+<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
+not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
+re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
+will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Untrusted data passed to external API
+ * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
+ * @id cpp/untrusted-data-to-external-api-ir
+ * @kind path-problem
+ * @precision low
+ * @problem.severity error
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import ir.ExternalAPIs
+import semmle.code.cpp.security.FlowSources
+import DataFlow::PathGraph
+
+from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
+    " with untrusted data from $@.", source, source.getNode().(RemoteFlowSource).getSourceType()

cpp/ql/src/Security/CWE/CWE-020/SafeExternalAPIFunction.qll

@@ -0,0 +1,24 @@
+/**
+ * Provides a class for modeling external functions that are "safe" from a security perspective.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.SideEffect
+
+/**
+ * A `Function` that is considered a "safe" external API from a security perspective.
+ */
+abstract class SafeExternalAPIFunction extends Function { }
+
+/** The default set of "safe" external APIs. */
+private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
+  DefaultSafeExternalAPIFunction() {
+    // If a function does not write to any of its arguments, we consider it safe to
+    // pass untrusted data to it. This means that string functions such as `strcmp`
+    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
+    exists(SideEffectFunction model | model = this |
+      model.hasOnlySpecificWriteSideEffects() and
+      not model.hasSpecificWriteSideEffect(_, _, _)
+    )
+  }
+}

cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.qhelp

@@ -0,0 +1,59 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports 
+external APIs that use untrusted data. The results are not filtered, so you can audit all examples.
+The query provides data for security reviews of the application and you can also use it to identify external APIs
+that should be modeled as either taint steps, or sinks for specific problems.</p>
+
+<p>An external API is defined as a call to a function that is not defined in the source code, and is not modeled
+as a taint step in the default taint library. External APIs may be from the
+C++ standard library, third-party dependencies or from internal dependencies. The query reports uses of 
+untrusted data in either the qualifier or as one of the arguments of external APIs.</p>
+
+</overview>
+<recommendation>
+
+<p>For each result:</p>
+
+<ul>
+  <li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or 
+  that the result is a false positive because this data is sanitized.</li>
+  <li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query, 
+  and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
+  <li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and 
+  re-run the query to determine what new results have appeared due to this additional modeling.</li>
+</ul>
+
+<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIFunction</code> 
+class to exclude known safe external APIs from future analysis.</p>
+
+</recommendation>
+<example>
+
+<p>In this first example, input is read from <code>fgets</code> and then ultimately used in a call to the 
+<code>fputs</code> external API:</p>
+
+<sample src="ExternalAPISinkExample.cpp" />
+
+<p>This is an XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled, 
+and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to 
+some existing sanitization.</p>
+
+<p>In this second example, again a request parameter is read from <code>fgets</code>.</p>
+
+<sample src="ExternalAPITaintStepExample.cpp" />
+
+<p>If the query reported the call to <code>strcat</code> on line 9, this would suggest that this external API is 
+not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
+re-run the query to determine what additional results might be found. In this example, it seems likely that <code>buffer</code>
+will be executed as an SQL query, potentially leading to an SQL injection vulnerability.</p>
+
+<p>Note that both examples are correctly handled by the standard taint tracking library and XSS query.</p>
+</example>
+<references>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Untrusted data passed to external API
+ * @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
+ * @id cpp/untrusted-data-to-external-api
+ * @kind path-problem
+ * @precision low
+ * @problem.severity error
+ * @tags security external/cwe/cwe-20
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.TaintTracking
+import ExternalAPIs
+import DataFlow::PathGraph
+
+from UntrustedDataToExternalAPIConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Call to " + sink.getNode().(ExternalAPIDataNode).getExternalFunction().toString() +
+    " with untrusted data from $@.", source, source.toString()

cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll

@@ -0,0 +1,50 @@
+/**
+ * Definitions for reasoning about untrusted data used in APIs defined outside the
+ * database.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.Taint
+import ExternalAPIsSpecific
+
+/** A node representing untrusted data being passed to an external API. */
+class UntrustedExternalAPIDataNode extends ExternalAPIDataNode {
+  UntrustedExternalAPIDataNode() { any(UntrustedDataToExternalAPIConfig c).hasFlow(_, this) }
+
+  /** Gets a source of untrusted data which is passed to this external API data node. */
+  DataFlow::Node getAnUntrustedSource() {
+    any(UntrustedDataToExternalAPIConfig c).hasFlow(result, this)
+  }
+}
+
+private newtype TExternalAPI =
+  TExternalAPIParameter(Function f, int index) {
+    exists(UntrustedExternalAPIDataNode n |
+      f = n.getExternalFunction() and
+      index = n.getIndex()
+    )
+  }
+
+/** An external API which is used with untrusted data. */
+class ExternalAPIUsedWithUntrustedData extends TExternalAPI {
+  /** Gets a possibly untrusted use of this external API. */
+  UntrustedExternalAPIDataNode getUntrustedDataNode() {
+    this = TExternalAPIParameter(result.getExternalFunction(), result.getIndex())
+  }
+
+  /** Gets the number of untrusted sources used with this external API. */
+  int getNumberOfUntrustedSources() {
+    result = strictcount(getUntrustedDataNode().getAnUntrustedSource())
+  }
+
+  /** Gets a textual representation of this element. */
+  string toString() {
+    exists(Function f, int index, string indexString |
+      if index = -1 then indexString = "qualifier" else indexString = "param " + index
+    |
+      this = TExternalAPIParameter(f, index) and
+      result = f.toString() + " [" + indexString + "]"
+    )
+  }
+}

cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIsSpecific.qll

@@ -0,0 +1,51 @@
+/**
+ * Provides IR-specific definitions for use in the `ExternalAPI` library.
+ */
+
+import semmle.code.cpp.ir.dataflow.TaintTracking
+private import semmle.code.cpp.security.FlowSources
+private import semmle.code.cpp.models.interfaces.DataFlow
+import SafeExternalAPIFunction
+
+/** A node representing untrusted data being passed to an external API. */
+class ExternalAPIDataNode extends DataFlow::Node {
+  Call call;
+  int i;
+
+  ExternalAPIDataNode() {
+    // Argument to call to a function
+    (
+      this.asExpr() = call.getArgument(i)
+      or
+      i = -1 and this.asExpr() = call.getQualifier()
+    ) and
+    exists(Function f |
+      f = call.getTarget() and
+      // Defined outside the source archive
+      not f.hasDefinition() and
+      // Not already modeled as a dataflow or taint step
+      not f instanceof DataFlowFunction and
+      not f instanceof TaintFunction and
+      // Not a call to a known safe external API
+      not f instanceof SafeExternalAPIFunction
+    )
+  }
+
+  /** Gets the called API `Function`. */
+  Function getExternalFunction() { result = call.getTarget() }
+
+  /** Gets the index which is passed untrusted data (where -1 indicates the qualifier). */
+  int getIndex() { result = i }
+
+  /** Gets the description of the function being called. */
+  string getFunctionDescription() { result = getExternalFunction().toString() }
+}
+
+/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalAPIDataNode`s. */
+class UntrustedDataToExternalAPIConfig extends TaintTracking::Configuration {
+  UntrustedDataToExternalAPIConfig() { this = "UntrustedDataToExternalAPIConfigIR" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalAPIDataNode }
+}

cpp/ql/src/Security/CWE/CWE-020/ir/SafeExternalAPIFunction.qll

@@ -0,0 +1,24 @@
+/**
+ * Provides a class for modeling external functions that are "safe" from a security perspective.
+ */
+
+private import cpp
+private import semmle.code.cpp.models.interfaces.SideEffect
+
+/**
+ * A `Function` that is considered a "safe" external API from a security perspective.
+ */
+abstract class SafeExternalAPIFunction extends Function { }
+
+/** The default set of "safe" external APIs. */
+private class DefaultSafeExternalAPIFunction extends SafeExternalAPIFunction {
+  DefaultSafeExternalAPIFunction() {
+    // If a function does not write to any of its arguments, we consider it safe to
+    // pass untrusted data to it. This means that string functions such as `strcmp`
+    // and `strlen`, as well as memory functions such as `memcmp`, are considered safe.
+    exists(SideEffectFunction model | model = this |
+      model.hasOnlySpecificWriteSideEffects() and
+      not model.hasSpecificWriteSideEffect(_, _, _)
+    )
+  }
+}

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

@@ -29,6 +29,8 @@ class QueryString extends EnvironmentRead {
 }
 
 class Configuration extends TaintTrackingConfiguration {
+  override predicate isSource(Expr source) { source instanceof QueryString }
+
   override predicate isSink(Element tainted) {
     exists(PrintStdoutCall call | call.getAnArgument() = tainted)
   }

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -27,6 +27,10 @@ class Configuration extends TaintTrackingConfiguration {
   override predicate isSink(Element tainted) {
     exists(SQLLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _))
   }
+
+  override predicate isBarrier(Expr e) {
+    super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
+  }
 }
 
 from

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -0,0 +1,5 @@
+unsigned limit = get_limit();
+unsigned total = 0;
+while (limit - total > 0) { // wrong: if `total` is greater than `limit` this will underflow and continue executing the loop.
+  total += get_data();
+}

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+This rule finds relational comparisons between the result of an unsigned subtraction and the value <code>0</code>.
+Such comparisons are likely to be wrong as the value of an unsigned subtraction can never be negative. So the
+relational comparison ends up checking whether the result of the subtraction is equal to <code>0</code>.
+This is probably not what the programmer intended.
+</p>
+</overview>
+<recommendation>
+
+<p>If a relational comparison is intended, consider casting the result of the subtraction to a signed type.
+   If the intention was to test for equality, consider replacing the relational comparison with an equality test.
+</p>
+
+</recommendation>
+<example>
+<sample src="UnsignedDifferenceExpressionComparedZero.c" />
+
+</example>
+<references>
+
+<li>SEI CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules">INT02-C. Understand integer conversion rules</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -0,0 +1,49 @@
+/**
+ * @name Unsigned difference expression compared to zero
+ * @description A subtraction with an unsigned result can never be negative. Using such an expression in a relational comparison with `0` is likely to be wrong.
+ * @kind problem
+ * @id cpp/unsigned-difference-expression-compared-zero
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       correctness
+ *       external/cwe/cwe-191
+ */
+
+import cpp
+import semmle.code.cpp.commons.Exclusions
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.controlflow.Guards
+
+/** Holds if `sub` is guarded by a condition which ensures that `left >= right`. */
+pragma[noinline]
+predicate isGuarded(SubExpr sub, Expr left, Expr right) {
+  exists(GuardCondition guard |
+    guard.controls(sub.getBasicBlock(), true) and
+    guard.ensuresLt(left, right, 0, sub.getBasicBlock(), false)
+  )
+}
+
+/** Holds if `sub` will never be negative. */
+predicate nonNegative(SubExpr sub) {
+  not exprMightOverflowNegatively(sub.getFullyConverted())
+  or
+  // The subtraction is guarded by a check of the form `left >= right`.
+  exists(GVN left, GVN right |
+    // This is basically a poor man's version of a directional unbind operator.
+    strictcount([left, globalValueNumber(sub.getLeftOperand())]) = 1 and
+    strictcount([right, globalValueNumber(sub.getRightOperand())]) = 1 and
+    isGuarded(sub, left.getAnExpr(), right.getAnExpr())
+  )
+}
+
+from RelationalOperation ro, SubExpr sub
+where
+  not isFromMacroDefinition(ro) and
+  not isFromMacroDefinition(sub) and
+  ro.getLesserOperand().getValue().toInt() = 0 and
+  ro.getGreaterOperand() = sub and
+  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
+  not nonNegative(sub)
+select ro, "Unsigned subtraction can never be negative."

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -34,6 +34,10 @@ predicate sqlite_encryption_used() {
 }
 
 class Configuration extends TaintTrackingConfiguration {
+  override predicate isSource(Expr source) {
+    super.isSource(source) and source instanceof SensitiveExpr
+  }
+
   override predicate isSink(Element taintedArg) {
     exists(SqliteFunctionCall sqliteCall |
       taintedArg = sqliteCall.getASource() and

cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll

@@ -189,8 +189,7 @@ class InitializationFunction extends Function {
       // Field wise assignment to the parameter
       any(Assignment e).getLValue() = getAFieldAccess(this.getParameter(i)) or
       i =
-        this
-            .(MemberFunction)
+        this.(MemberFunction)
             .getAnOverridingFunction+()
             .(InitializationFunction)
             .initializedParameter() or
@@ -327,52 +326,37 @@ class InitializationFunction extends Function {
       // Return value is not a success code but the output functions never fail.
       name.matches("_Interlocked%")
       or
-      // Functions that never fail, according to MSDN.
-      name = "QueryPerformanceCounter"
-      or
-      name = "QueryPerformanceFrequency"
-      or
-      // Functions that never fail post-Vista, according to MSDN.
-      name = "InitializeCriticalSectionAndSpinCount"
-      or
-      // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN.
-      name = "rand_s"
-      or
-      // IntersectRect initializes the argument regardless of whether the input intersects
-      name = "IntersectRect"
-      or
-      name = "SetRect"
-      or
-      name = "UnionRect"
-      or
-      // These functions appears to have an incorrect CFG, which leads to false positives
-      name = "PhysicalToLogicalDPIPoint"
-      or
-      name = "LogicalToPhysicalDPIPoint"
-      or
-      // Sets NtProductType to default on error
-      name = "RtlGetNtProductType"
-      or
-      // Our CFG is not sophisticated enough to detect that the argument is always initialized
-      name = "StringCchLengthA"
-      or
-      // All paths init the argument, and always returns SUCCESS.
-      name = "RtlUnicodeToMultiByteSize"
-      or
-      // All paths init the argument, and always returns SUCCESS.
-      name = "RtlMultiByteToUnicodeSize"
-      or
-      // All paths init the argument, and always returns SUCCESS.
-      name = "RtlUnicodeToMultiByteN"
-      or
-      // Always initializes argument
-      name = "RtlGetFirstRange"
-      or
-      // Destination range is zeroed out on failure, assuming first two parameters are valid
-      name = "memcpy_s"
-      or
-      // This zeroes the memory unconditionally
-      name = "SeCreateAccessState"
+      name =
+        [
+          // Functions that never fail, according to MSDN.
+          "QueryPerformanceCounter", "QueryPerformanceFrequency",
+          // Functions that never fail post-Vista, according to MSDN.
+          "InitializeCriticalSectionAndSpinCount",
+          // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN.
+          "rand_s",
+          // IntersectRect initializes the argument regardless of whether the input intersects
+          "IntersectRect", "SetRect", "UnionRect",
+          // These functions appears to have an incorrect CFG, which leads to false positives
+          "PhysicalToLogicalDPIPoint", "LogicalToPhysicalDPIPoint",
+          // Sets NtProductType to default on error
+          "RtlGetNtProductType",
+          // Our CFG is not sophisticated enough to detect that the argument is always initialized
+          "StringCchLengthA",
+          // All paths init the argument, and always returns SUCCESS.
+          "RtlUnicodeToMultiByteSize",
+          // All paths init the argument, and always returns SUCCESS.
+          "RtlMultiByteToUnicodeSize",
+          // All paths init the argument, and always returns SUCCESS.
+          "RtlUnicodeToMultiByteN",
+          // Always initializes argument
+          "RtlGetFirstRange",
+          // Destination range is zeroed out on failure, assuming first two parameters are valid
+          "memcpy_s",
+          // This zeroes the memory unconditionally
+          "SeCreateAccessState",
+          // Argument initialization is optional, but always succeeds
+          "KeGetCurrentProcessorNumberEx"
+        ]
     )
   }
 }
@@ -475,12 +459,9 @@ class ConditionalInitializationCall extends FunctionCall {
       fa.getASuccessor+() = result
     ) and
     result =
-      this
-          .getArgument(getTarget(this)
-                .(ConditionalInitializationFunction)
-                .conditionallyInitializedParameter(_))
-          .(AddressOfExpr)
-          .getOperand()
+      this.getArgument(getTarget(this)
+            .(ConditionalInitializationFunction)
+            .conditionallyInitializedParameter(_)).(AddressOfExpr).getOperand()
   }
 
   Variable getStatusVariable() {

cpp/ql/src/Security/CWE/CWE-732/FilePermissions.qll

@@ -140,12 +140,9 @@ class FopenCreationExpr extends FileCreationExpr {
 
 class FopensCreationExpr extends FileCreationExpr {
   FopensCreationExpr() {
-    exists(string name | name = this.getTarget().getName() |
-      name = "fopen_s" or
-      name = "_wfopen_s"
-    ) and
+    this.getTarget().getName() = ["fopen_s", "_wfopen_s"] and
     exists(string mode |
-      (mode = "w" or mode = "a") and
+      mode = ["w", "a"] and
       this.getArgument(2).getValue().matches(mode + "%")
     )
   }

cpp/ql/src/experimental/Security/CWE/CWE-120/MemoryUnsafeFunctionScan.cpp

@@ -13,9 +13,10 @@ int main(int argc, char **argv)
     char buf1[10];
     scanf("%s", buf1);
 
-    // GOOD, length is specified. The length should be one less than the size of the buffer, since the last character is the NULL terminator.
-    char buf2[10];
-    sscanf(buf2, "%9s");
+    // GOOD, length is specified. The length should be one less than the size of the destination buffer, since the last character is the NULL terminator.
+    char buf2[20];
+    char buf3[10];
+    sscanf(buf2, "%9s", buf3);
 
     // BAD, do not use scanf without specifying a length first
     char file[10];

cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.c

@@ -0,0 +1,35 @@
+// BAD: the memset call will probably be removed.
+void getPassword(void) {
+  char pwd[64];
+  if (GetPassword(pwd, sizeof(pwd))) {
+    /* Checking of password, secure operations, etc. */
+  }
+  memset(pwd, 0, sizeof(pwd));
+}
+// GOOD: in this case the memset will not be removed.
+void getPassword(void) {
+  char pwd[64];
+ 
+  if (retrievePassword(pwd, sizeof(pwd))) {
+     /* Checking of password, secure operations, etc. */
+  }
+  memset_s(pwd, 0, sizeof(pwd));
+}
+// GOOD: in this case the memset will not be removed.
+void getPassword(void) {
+  char pwd[64];
+  if (retrievePassword(pwd, sizeof(pwd))) {
+    /* Checking of password, secure operations, etc. */
+  }
+  SecureZeroMemory(pwd, sizeof(pwd));
+}
+// GOOD: in this case the memset will not be removed.
+void getPassword(void) {
+  char pwd[64];
+  if (retrievePassword(pwd, sizeof(pwd))) {
+    /* Checking of password, secure operations, etc. */
+  }
+#pragma optimize("", off)
+  memset(pwd, 0, sizeof(pwd));
+#pragma optimize("", on)
+}

cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Compiler optimization will exclude the cleaning of private information.
+Using the <code>memset</code> function to clear private data in a variable that has no subsequent use is potentially dangerous, since the compiler can remove the call.
+For some compilers, optimization is also possible when using calls to free memory after the <code>memset</code> function.</p>
+
+<p>It is possible to miss detection of vulnerabilities if used to clear fields of structures or parts of a buffer.</p>
+
+</overview>
+<recommendation>
+
+<p>We recommend to use the <code>RtlSecureZeroMemory</code> or <code>memset_s</code> functions, or compilation flags that exclude optimization of <code>memset</code> calls (e.g. -fno-builtin-memset).</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the <code>memset</code> function.</p>
+<sample src="CompilerRemovalOfCodeToClearBuffers.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations">MSC06-C. Beware of compiler optimizations</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-14/CompilerRemovalOfCodeToClearBuffers.ql

@@ -0,0 +1,127 @@
+/**
+ * @name Compiler Removal Of Code To Clear Buffers
+ * @description Using <code>memset</code> the function to clear private data in a variable that has no subsequent use
+ *              is potentially dangerous because the compiler can remove the call.
+ * @kind problem
+ * @id cpp/compiler-removal-of-code-to-clear-buffers
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       external/cwe/cwe-14
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.dataflow.StackAddress
+
+/**
+ * A call to `memset` of the form `memset(ptr, value, num)`, for some local variable `ptr`.
+ */
+class CompilerRemovaMemset extends FunctionCall {
+  CompilerRemovaMemset() {
+    this.getTarget().hasGlobalOrStdName("memset") and
+    exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
+      DataFlow::localFlow(source, sink) and
+      this.getArgument(0) = isv.getAnAccess() and
+      (
+        source.asExpr() = exp
+        or
+        // handle the case where exp is defined by an address being passed into some function.
+        source.asDefiningArgument() = exp
+      ) and
+      exp.getLocation().getEndLine() < this.getArgument(0).getLocation().getStartLine() and
+      sink.asExpr() = this.getArgument(0)
+    )
+  }
+
+  predicate isExistsAllocForThisVariable() {
+    exists(AllocationExpr alloc, Variable v |
+      alloc = v.getAnAssignedValue() and
+      this.getArgument(0) = v.getAnAccess() and
+      alloc.getASuccessor+() = this
+    )
+    or
+    not stackPointerFlowsToUse(this.getArgument(0), _, _, _)
+  }
+
+  predicate isExistsFreeForThisVariable() {
+    exists(DeallocationExpr free, Variable v |
+      this.getArgument(0) = v.getAnAccess() and
+      free.getFreedExpr() = v.getAnAccess() and
+      this.getASuccessor+() = free
+    )
+  }
+
+  predicate isExistsCallWithThisVariableExcludingDeallocationCalls() {
+    exists(FunctionCall fc, Variable v |
+      not fc instanceof DeallocationExpr and
+      this.getArgument(0) = v.getAnAccess() and
+      fc.getAnArgument() = v.getAnAccess() and
+      this.getASuccessor+() = fc
+    )
+  }
+
+  predicate isVariableUseAfterMemsetExcludingCalls() {
+    exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Expr exp |
+      DataFlow::localFlow(source, sink) and
+      this.getArgument(0) = isv.getAnAccess() and
+      source.asExpr() = isv.getAnAccess() and
+      exp.getLocation().getStartLine() > this.getArgument(2).getLocation().getEndLine() and
+      not exp.getParent() instanceof FunctionCall and
+      sink.asExpr() = exp
+    )
+  }
+
+  predicate isVariableUseBoundWithArgumentFunction() {
+    exists(DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, Parameter p, Expr exp |
+      DataFlow::localFlow(source, sink) and
+      this.getArgument(0) = isv.getAnAccess() and
+      this.getEnclosingFunction().getAParameter() = p and
+      exp.getAChild*() = p.getAnAccess() and
+      source.asExpr() = exp and
+      sink.asExpr() = isv.getAnAccess()
+    )
+  }
+
+  predicate isVariableUseBoundWithGlobalVariable() {
+    exists(
+      DataFlow::Node source, DataFlow::Node sink, LocalVariable isv, GlobalVariable gv, Expr exp
+    |
+      DataFlow::localFlow(source, sink) and
+      this.getArgument(0) = isv.getAnAccess() and
+      exp.getAChild*() = gv.getAnAccess() and
+      source.asExpr() = exp and
+      sink.asExpr() = isv.getAnAccess()
+    )
+  }
+
+  predicate isExistsCompilationFlagsBlockingRemoval() {
+    exists(Compilation c |
+      c.getAFileCompiled() = this.getFile() and
+      c.getAnArgument() = "-fno-builtin-memset"
+    )
+  }
+
+  predicate isUseVCCompilation() {
+    exists(Compilation c |
+      c.getAFileCompiled() = this.getFile() and
+      (
+        c.getArgument(2).matches("%gcc%") or
+        c.getArgument(2).matches("%g++%") or
+        c.getArgument(2).matches("%clang%") or
+        c.getArgument(2) = "--force-recompute"
+      )
+    )
+  }
+}
+
+from CompilerRemovaMemset fc
+where
+  not (fc.isExistsAllocForThisVariable() and not fc.isExistsFreeForThisVariable()) and
+  not (fc.isExistsFreeForThisVariable() and not fc.isUseVCCompilation()) and
+  not fc.isVariableUseAfterMemsetExcludingCalls() and
+  not fc.isExistsCallWithThisVariableExcludingDeallocationCalls() and
+  not fc.isVariableUseBoundWithArgumentFunction() and
+  not fc.isVariableUseBoundWithGlobalVariable() and
+  not fc.isExistsCompilationFlagsBlockingRemoval()
+select fc.getArgument(0), "This variable will not be cleared."

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.cpp

@@ -0,0 +1,19 @@
+
+image::image(int width, int height)
+{
+	int x, y;
+
+	// allocate width * height pixels
+	pixels = new uint32_t[width * height];
+
+	// fill width * height pixels
+	for (y = 0; y < height; y++)
+	{
+		for (x = 0; x < width; x++)
+		{
+			pixels[(y * width) + height] = 0;
+		}
+	}
+
+	// ...
+}

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.qhelp

@@ -0,0 +1,25 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>The result of a multiplication is used in the size of an allocation. If the multiplication can be made to overflow, a much smaller amount of memory may be allocated than the rest of the code expects. This may lead to overflowing writes when the buffer is accessed later.</p>
+</overview>
+
+<recommendation>
+<p>To fix this issue, ensure that the arithmetic used in the size of an allocation cannot overflow before memory is allocated.</p>
+</recommendation>
+
+<example>
+<p>In the following example, an array of size <code>width * height</code> is allocated and stored as <code>pixels</code>. If <code>width</code> and <code>height</code> are set such that the multiplication overflows and wraps to a small value (say, 4) then the initialization code that follows the allocation will write beyond the end of the array.</p>
+<sample src="AllocMultiplicationOverflow.cpp"/>
+</example>
+
+<references>
+<li>
+  Cplusplus.com: <a href="http://www.cplusplus.com/articles/DE18T05o/">Integer overflow</a>.
+</li>
+</references>
+
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.ql

@@ -0,0 +1,40 @@
+/**
+ * @name Multiplication result may overflow and be used in allocation
+ * @description Using a multiplication result that may overflow in the size of an allocation may lead to buffer overflows when the allocated memory is used.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision low
+ * @tags security
+ *       correctness
+ *       external/cwe/cwe-190
+ *       external/cwe/cwe-128
+ * @id cpp/multiplication-overflow-in-alloc
+ */
+
+import cpp
+import semmle.code.cpp.models.interfaces.Allocation
+import semmle.code.cpp.dataflow.DataFlow
+import DataFlow::PathGraph
+
+class MultToAllocConfig extends DataFlow::Configuration {
+  MultToAllocConfig() { this = "MultToAllocConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    // a multiplication of two non-constant expressions
+    exists(MulExpr me |
+      me = node.asExpr() and
+      forall(Expr e | e = me.getAnOperand() | not exists(e.getValue()))
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    // something that affects an allocation size
+    node.asExpr() = any(AllocationExpr ae).getSizeExpr().getAChild*()
+  }
+}
+
+from MultToAllocConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Potentially overflowing value from $@ is used in the size of this allocation.", source,
+  "multiplication"

cpp/ql/src/experimental/Security/CWE/CWE-359/PrivateCleartextWrite.ql

@@ -16,6 +16,6 @@ import DataFlow::PathGraph
 
 from WriteConfig b, DataFlow::PathNode source, DataFlow::PathNode sink
 where b.hasFlowPath(source, sink)
-select sink.getNode(),
-  "This write into the external location '" + sink + "' may contain unencrypted data from $@",
-  source, "this source."
+select sink.getNode(), source, sink,
+  "This write into the external location '" + sink.getNode() +
+    "' may contain unencrypted data from $@", source, "this source."

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.c

@@ -0,0 +1,20 @@
+// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+if (currentSize < newSize)
+{
+	buffer = (unsigned char *)realloc(buffer, newSize);
+}
+
+
+
+// GOOD: this way we will exclude possible memory leak 
+unsigned char * tmp;
+if (currentSize < newSize)
+{
+	tmp = (unsigned char *)realloc(buffer, newSize);
+}
+if (tmp == NULL)
+{
+	free(buffer);
+} 
+else
+	buffer = tmp;

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.qhelp

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Memory leak on failed call to realloc. 
+The expression <code>mem = realloc (mem, size)</code> is potentially dangerous, if the call fails, we will lose the pointer to the memory block.  
+An unsuccessful call is possible not only when trying to allocate a large amount of memory, but also when the process memory is strongly segmented.</p>
+
+<p>False positives include code in which immediately after calling the realloc function, the pointer is manipulated without first checking for validity. 
+In this case, an exception will occur in the program and it will terminate. 
+But from the point of view of safe coding, these places require the attention of developers.
+At this stage, false positives are also possible in situations where the exception handling is quite complicated and occurs outside the base block in which memory is redistributed.</p>
+
+</overview>
+<recommendation>
+
+<p>We recommend storing the result in a temporary variable and eliminating memory leak.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the <code>realloc</code> function.</p>
+<sample src="MemoryLeakOnFailedCallToRealloc.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C++ Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM51-CPP.+Properly+deallocate+dynamically+allocated+resources">MEM51-CPP. Properly deallocate dynamically allocated resources</a>.
+</li>
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/WIN30-C.+Properly+pair+allocation+and+deallocation+functions">WIN30-C. Properly pair allocation and deallocation functions</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.ql

@@ -0,0 +1,61 @@
+/**
+ * @name Memory leak on failed call to realloc
+ * @description The expression mem = realloc (mem, size) is potentially dangerous, if the call fails, we will lose the pointer to the memory block.
+ *              We recommend storing the result in a temporary variable and eliminating memory leak.
+ * @kind problem
+ * @id cpp/memory-leak-on-failed-call-to-realloc
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-401
+ */
+
+import cpp
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.valuenumbering.HashCons
+
+/**
+ * A function call that potentially does not return (such as `exit`).
+ */
+class CallMayNotReturn extends FunctionCall {
+  CallMayNotReturn() {
+    // call that is known to not return
+    not exists(this.(ControlFlowNode).getASuccessor())
+    or
+    // call to another function that may not return
+    exists(CallMayNotReturn exit | getTarget() = exit.getEnclosingFunction())
+  }
+}
+
+/**
+ * A call to `realloc` of the form `v = realloc(v, size)`, for some variable `v`.
+ */
+class ReallocCallLeak extends FunctionCall {
+  Variable v;
+
+  ReallocCallLeak() {
+    exists(AssignExpr ex |
+      this.getTarget().hasGlobalOrStdName("realloc") and
+      this = ex.getRValue() and
+      hashCons(ex.getLValue()) = hashCons(this.getArgument(0)) and
+      v.getAnAccess() = this.getArgument(0)
+    )
+  }
+
+  /**
+   * Holds if failure of this allocation may be handled by termination, for
+   * example a call to `exit()`.
+   */
+  predicate mayHandleByTermination() {
+    exists(GuardCondition guard, CallMayNotReturn exit |
+      this.(ControlFlowNode).getASuccessor*() = guard and
+      guard.getAChild*() = v.getAnAccess() and
+      guard.controls(exit.getBasicBlock(), _)
+    )
+  }
+}
+
+from ReallocCallLeak rcl
+where not rcl.mayHandleByTermination()
+select rcl, "possible loss of original pointer on unsuccessful call realloc"

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp

@@ -0,0 +1,35 @@
+// BAD: on memory allocation error, the program terminates.
+void badFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new int[length];
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// BAD: memory allocation error will not be handled.
+void badFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new (std::nothrow) int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new (std::nothrow) int[length];
+  if (!dest) {
+      return;
+  }
+  std::memset(dest, 0, length);
+// ..
+}

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>When using the <code>new</code> operator to allocate memory, you need to pay attention to the different ways of detecting errors. <code>::operator new(std::size_t)</code> throws an exception on error, whereas <code>::operator new(std::size_t, const std::nothrow_t &amp;)</code> returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.</p>
+
+</overview>
+<recommendation>
+
+<p>Use the correct error detection method corresponding with the memory allocation.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates various approaches to detecting memory allocation errors using the <code>new</code> operator.</p>
+<sample src="WrongInDetectingAndHandlingMemoryAllocationErrors.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C++ Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

@@ -0,0 +1,87 @@
+/**
+ * @name Detect And Handle Memory Allocation Errors
+ * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error.
+ *              --the programmer can get confused when check the error that occurs when allocating memory incorrectly.
+ * @kind problem
+ * @id cpp/detect-and-handle-memory-allocation-errors
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-570
+ */
+
+import cpp
+
+/**
+ * Lookup if condition compare with 0
+ */
+class IfCompareWithZero extends IfStmt {
+  IfCompareWithZero() {
+    this.getCondition().(EQExpr).getAChild().getValue() = "0"
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.hasElse()
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.getThen().getAChild*() instanceof ReturnStmt
+  }
+}
+
+/**
+ * lookup for calls to `operator new`, with incorrect error handling.
+ */
+class WrongCheckErrorOperatorNew extends FunctionCall {
+  Expr exp;
+
+  WrongCheckErrorOperatorNew() {
+    this = exp.(NewOrNewArrayExpr).getAChild().(FunctionCall) and
+    (
+      this.getTarget().hasGlobalOrStdName("operator new")
+      or
+      this.getTarget().hasGlobalOrStdName("operator new[]")
+    )
+  }
+
+  /**
+   * Holds if handler `try ... catch` exists.
+   */
+  predicate isExistsTryCatchBlock() {
+    exists(TryStmt ts | this.getEnclosingStmt() = ts.getStmt().getAChild*())
+  }
+
+  /**
+   * Holds if results call `operator new` check in `operator if`.
+   */
+  predicate isExistsIfCondition() {
+    exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it |
+      // call `operator new` directly from the condition of `operator if`.
+      this = ifc.getCondition().getAChild*()
+      or
+      // check results call `operator new` with variable appropriation
+      postDominates(ifc, this) and
+      aex.getAChild() = exp and
+      ifc.getCondition().getAChild().(VariableAccess).getTarget() =
+        aex.getLValue().(VariableAccess).getTarget()
+      or
+      // check results call `operator new` with declaration variable
+      postDominates(ifc, this) and
+      exp = it.getExpr() and
+      it.getDeclaration() = ifc.getCondition().getAChild().(VariableAccess).getTarget()
+    )
+  }
+
+  /**
+   * Holds if `(std::nothrow)` exists in call `operator new`.
+   */
+  predicate isExistsNothrow() { this.getAChild().toString() = "nothrow" }
+}
+
+from WrongCheckErrorOperatorNew op
+where
+  // use call `operator new` with `(std::nothrow)` and checking error using `try ... catch` block and not `operator if`
+  op.isExistsNothrow() and not op.isExistsIfCondition() and op.isExistsTryCatchBlock()
+  or
+  // use call `operator new` without `(std::nothrow)` and checking error using `operator if` and not  `try ... catch` block
+  not op.isExistsNothrow() and not op.isExistsTryCatchBlock() and op.isExistsIfCondition()
+select op, "memory allocation error check is incorrect or missing"

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.c

@@ -0,0 +1,9 @@
+// BAD: if buffer does not have a terminal zero, then access outside the allocated memory is possible.
+
+buffer[strlen(buffer)] = 0;
+
+
+// GOOD: we will eliminate dangerous behavior if we use a different method of calculating the length. 
+size_t len;
+...
+buffer[len] = 0

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.qhelp

@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Potentially dangerous use of the strlen function to calculate the length of a string.
+The expression <code>buffer[strlen(buffer)] = 0</code> is potentially dangerous, if the variable buffer does not have a terminal zero, then access beyond the bounds of the allocated memory is possible, which will lead to undefined behavior.
+If terminal zero is present, then the specified expression is meaningless.</p>
+
+<p>False positives include heavily nested strlen. This situation is unlikely.</p>
+
+</overview>
+<recommendation>
+
+<p>We recommend using another method for calculating the string length</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the strlen function.</p>
+<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/STR32-C.+Do+not+pass+a+non-null-terminated+character+sequence+to+a+library+function+that+expects+a+string">STR32-C. Do not pass a non-null-terminated character sequence to a library function that expects a string</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrlen.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Access Of Memory Location After End Of Buffer
+ * @description The expression `buffer [strlen (buffer)] = 0` is potentially dangerous, if the variable `buffer` does not have a terminal zero, then access beyond the bounds of the allocated memory is possible, which will lead to undefined behavior.
+ *              If terminal zero is present, then the specified expression is meaningless.
+ * @kind problem
+ * @id cpp/access-memory-location-after-end-buffer
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-788
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.dataflow.DataFlow
+
+from StrlenCall fc, AssignExpr expr, ArrayExpr exprarr
+where
+  exprarr = expr.getLValue() and
+  expr.getRValue().getValue().toInt() = 0 and
+  globalValueNumber(exprarr.getArrayOffset()) = globalValueNumber(fc) and
+  not exists(Expr exptmp |
+    (
+      DataFlow::localExprFlow(fc, exptmp) or
+      exptmp.getAChild*() = fc.getArgument(0).(VariableAccess).getTarget().getAnAccess()
+    ) and
+    dominates(exptmp, expr) and
+    postDominates(exptmp, fc) and
+    not exptmp.getEnclosingStmt() = fc.getEnclosingStmt() and
+    not exptmp.getEnclosingStmt() = expr.getEnclosingStmt()
+  ) and
+  globalValueNumber(fc.getArgument(0)) = globalValueNumber(exprarr.getArrayBase())
+select expr, "potential unsafe or redundant assignment."

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c

@@ -0,0 +1,4 @@
+ 
+strncat(dest, source, sizeof(dest) - strlen(dest)); // BAD: writes a zero byte past the `dest` buffer.
+
+strncat(dest, source, sizeof(dest) - strlen(dest) -1); // GOOD: Reserves space for the zero byte.

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.qhelp

@@ -0,0 +1,32 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The standard library function <code>strncat(dest, source, count)</code> appends the <code>source</code> string to the <code>dest</code> string. <code>count</code> specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form <code> strncat (dest, source, sizeof (dest) - strlen (dest)) </code> set the third argument to one more than possible. So when the <code>dest</code> is full, the expression <code> sizeof (dest) - strlen (dest) </code> will be equal to one, and not zero as the programmer might think. Making a call of this type may result in a zero byte being written just outside the <code>dest</code> buffer.</p>
+
+
+</overview>
+<recommendation>
+
+<p>We recommend subtracting one from the third argument. For example, replace <code>strncat(dest, source, sizeof(dest)-strlen(dest))</code> with <code>strncat(dest, source, sizeof(dest)-strlen(dest)-1)</code>.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the <code>strncat</code> function.</p>
+<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.
+</li>
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.ql

@@ -0,0 +1,64 @@
+/**
+ * @name Access Of Memory Location After The End Of A Buffer Using Strncat
+ * @description Calls of the form `strncat(dest, source, sizeof (dest) - strlen (dest))` set the third argument to one more than possible. So when `dest` is full, the expression `sizeof(dest) - strlen (dest)` will be equal to one, and not zero as the programmer might think. Making a call of this type may result in a zero byte being written just outside the `dest` buffer.
+ * @kind problem
+ * @id cpp/access-memory-location-after-end-buffer
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-788
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/**
+ * A call to `strncat` of the form `strncat(buff, str, someExpr - strlen(buf))`, for some expression `someExpr` equal to `sizeof(buff)`.
+ */
+class WrongCallStrncat extends FunctionCall {
+  Expr leftsomeExpr;
+
+  WrongCallStrncat() {
+    this.getTarget().hasGlobalOrStdName("strncat") and
+    // the expression of the first argument in `strncat` and `strnlen` is identical
+    globalValueNumber(this.getArgument(0)) =
+      globalValueNumber(this.getArgument(2).(SubExpr).getRightOperand().(StrlenCall).getStringExpr()) and
+    // using a string constant often speaks of manually calculating the length of the required buffer.
+    (
+      not this.getArgument(1) instanceof StringLiteral and
+      not this.getArgument(1) instanceof CharLiteral
+    ) and
+    // for use in predicates
+    leftsomeExpr = this.getArgument(2).(SubExpr).getLeftOperand()
+  }
+
+  /**
+   * Holds if the left side of the expression `someExpr` equal to `sizeof(buf)`.
+   */
+  predicate isExpressionEqualSizeof() {
+    // the left side of the expression `someExpr` is `sizeof(buf)`.
+    globalValueNumber(this.getArgument(0)) =
+      globalValueNumber(leftsomeExpr.(SizeofExprOperator).getExprOperand())
+    or
+    // value of the left side of the expression `someExpr` equal  `sizeof(buf)` value, and `buf` is array.
+    leftsomeExpr.getValue().toInt() = this.getArgument(0).getType().getSize()
+  }
+
+  /**
+   * Holds if the left side of the expression `someExpr` equal to variable containing the length of the memory allocated for the buffer.
+   */
+  predicate isVariableEqualValueSizegBuffer() {
+    // the left side of expression `someExpr` is the variable that was used in the function of allocating memory for the buffer`.
+    exists(AllocationExpr alc |
+      leftsomeExpr.(VariableAccess).getTarget() =
+        alc.(FunctionCall).getArgument(0).(VariableAccess).getTarget()
+    )
+  }
+}
+
+from WrongCallStrncat sc
+where
+  sc.isExpressionEqualSizeof() or
+  sc.isVariableEqualValueSizegBuffer()
+select sc, "if the used buffer is full, writing out of the buffer is possible"

cpp/ql/src/jsf/4.07 Header Files/AV Rule 35.ql

@@ -4,7 +4,7 @@
  *              the file from being included twice). This prevents errors and
  *              inefficiencies caused by repeated inclusion.
  * @kind problem
- * @problem.severity warning
+ * @problem.severity recommendation
  * @precision high
  * @id cpp/missing-header-guard
  * @tags efficiency

cpp/ql/src/jsf/4.09 Style/AV Rule 48.ql

@@ -31,8 +31,7 @@ predicate canonicalName1(Declaration d, string canonical) {
 
 predicate canonicalName2(Declaration d, string canonical) {
   canonical =
-    d
-        .getName()
+    d.getName()
         .replaceAll("_", "")
         .replaceAll("0", "O")
         .replaceAll("D", "O")

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

@@ -35,8 +35,7 @@ private predicate autogeneratedComment(string comment) {
         .regexpMatch("(?si).*(" +
             // replace `generated` with a regexp that also catches things like
             // `auto-generated`.
-            cond
-                .replaceAll("generated", "(auto[\\w-]*[\\s/\\*\\r\\n]*)?generated")
+            cond.replaceAll("generated", "(auto[\\w-]*[\\s/\\*\\r\\n]*)?generated")
                 // replace `!` with a regexp for end-of-sentence / separator characters.
                 .replaceAll("!", "[\\.\\?\\!\\-\\;\\,]")
                 // replace ` ` with a regexp for one or more whitespace characters

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

@@ -236,9 +236,8 @@ class Class extends UserType {
     or
     exists(ClassDerivation cd | cd.getBaseClass() = base |
       result =
-        this
-            .accessOfBaseMemberMulti(cd.getDerivedClass(),
-              fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
+        this.accessOfBaseMemberMulti(cd.getDerivedClass(),
+          fieldInBase.accessInDirectDerived(cd.getASpecifier().(AccessSpecifier)))
     )
   }
 
@@ -977,7 +976,12 @@ class ClassTemplateInstantiation extends Class {
  * specialization - see `FullClassTemplateSpecialization` and
  * `PartialClassTemplateSpecialization`).
  */
-abstract class ClassTemplateSpecialization extends Class {
+class ClassTemplateSpecialization extends Class {
+  ClassTemplateSpecialization() {
+    isFullClassTemplateSpecialization(this) or
+    isPartialClassTemplateSpecialization(this)
+  }
+
   /**
    * Gets the primary template for the specialization, for example on
    * `S<T,int>`, the result is `S<T,U>`.
@@ -997,6 +1001,16 @@ abstract class ClassTemplateSpecialization extends Class {
   override string getAPrimaryQlClass() { result = "ClassTemplateSpecialization" }
 }
 
+private predicate isFullClassTemplateSpecialization(Class c) {
+  // This class has template arguments, but none of them involves a template parameter.
+  exists(c.getATemplateArgument()) and
+  not exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
+  // This class does not have any instantiations.
+  not exists(c.(TemplateClass).getAnInstantiation()) and
+  // This class is not an instantiation of a class template.
+  not c instanceof ClassTemplateInstantiation
+}
+
 /**
  * A full specialization of a class template.  For example `MyTemplateClass<int>`
  * in the following code is a `FullClassTemplateSpecialization`:
@@ -1013,19 +1027,31 @@ abstract class ClassTemplateSpecialization extends Class {
  * ```
  */
 class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
-  FullClassTemplateSpecialization() {
-    // This class has template arguments, but none of them involves a template parameter.
-    exists(getATemplateArgument()) and
-    not exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
-    // This class does not have any instantiations.
-    not exists(this.(TemplateClass).getAnInstantiation()) and
-    // This class is not an instantiation of a class template.
-    not this instanceof ClassTemplateInstantiation
-  }
+  FullClassTemplateSpecialization() { isFullClassTemplateSpecialization(this) }
 
   override string getAPrimaryQlClass() { result = "FullClassTemplateSpecialization" }
 }
 
+private predicate isPartialClassTemplateSpecialization(Class c) {
+  /*
+   * (a) At least one of this class's template arguments involves a
+   *     template parameter in some respect, for example T, T*, etc.
+   *
+   * (b) It is not the case that the n template arguments of this class
+   *     are a set of n distinct template parameters.
+   *
+   * template <typename T,U> class X {};      // class template
+   * template <typename T> class X<T,T> {};   // partial class template specialization
+   * template <typename T> class X<T,int> {}; // partial class template specialization
+   * template <typename T> class Y {};        // class template
+   * template <typename T> class Y<T*> {};    // partial class template specialization
+   */
+
+  exists(Type ta | ta = c.getATemplateArgument() and ta.involvesTemplateParameter()) and
+  count(TemplateParameter tp | tp = c.getATemplateArgument()) !=
+    count(int i | exists(c.getTemplateArgument(i)))
+}
+
 /**
  * A partial specialization of a class template.  For example `MyTemplateClass<int, T>`
  * in the following code is a `PartialClassTemplateSpecialization`:
@@ -1042,25 +1068,7 @@ class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
  * ```
  */
 class PartialClassTemplateSpecialization extends ClassTemplateSpecialization {
-  PartialClassTemplateSpecialization() {
-    /*
-     * (a) At least one of this class's template arguments involves a
-     *     template parameter in some respect, for example T, T*, etc.
-     *
-     * (b) It is not the case that the n template arguments of this class
-     *     are a set of n distinct template parameters.
-     *
-     * template <typename T,U> class X {};      // class template
-     * template <typename T> class X<T,T> {};   // partial class template specialization
-     * template <typename T> class X<T,int> {}; // partial class template specialization
-     * template <typename T> class Y {};        // class template
-     * template <typename T> class Y<T*> {};    // partial class template specialization
-     */
-
-    exists(Type ta | ta = getATemplateArgument() and ta.involvesTemplateParameter()) and
-    count(TemplateParameter tp | tp = getATemplateArgument()) !=
-      count(int i | exists(getTemplateArgument(i)))
-  }
+  PartialClassTemplateSpecialization() { isPartialClassTemplateSpecialization(this) }
 
   override string getAPrimaryQlClass() { result = "PartialClassTemplateSpecialization" }
 }

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

@@ -50,5 +50,5 @@ class CStyleComment extends Comment {
  * ```
  */
 class CppStyleComment extends Comment {
-  CppStyleComment() { this.getContents().prefix(2) = "//" }
+  CppStyleComment() { this.getContents().matches("//%") }
 }

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

@@ -478,9 +478,8 @@ class AccessHolder extends Declaration, TAccessHolder {
    */
   pragma[inline]
   predicate canAccessMember(Declaration member, Class derived) {
-    this
-        .couldAccessMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier),
-          derived)
+    this.couldAccessMember(member.getDeclaringType(), member.getASpecifier().(AccessSpecifier),
+      derived)
   }
 
   /**

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

@@ -65,11 +65,10 @@ class ElementBase extends @element {
    * which they belong; for example, `AddExpr` is a primary class, but
    * `BinaryOperation` is not.
    *
-   * This predicate always has a result. If no primary class can be
-   * determined, the result is `"???"`. If multiple primary classes match,
-   * this predicate can have multiple results.
+   * This predicate can have multiple results if multiple primary classes match.
+   * For some elements, this predicate may not have a result.
    */
-  string getAPrimaryQlClass() { result = "???" }
+  string getAPrimaryQlClass() { none() }
 }
 
 /**
@@ -271,7 +270,12 @@ private predicate isFromUninstantiatedTemplateRec(Element e, Element template) {
 }
 
 /**
- * A C++11 `static_assert` or C11 `_Static_assert` construct.
+ * A C++11 `static_assert` or C11 `_Static_assert` construct. For example each
+ * line in the following example contains a static assert:
+ * ```
+ * static_assert(sizeof(MyStruct) <= 4096);
+ * static_assert(sizeof(MyStruct) <= 4096, "MyStruct is too big!");
+ * ```
  */
 class StaticAssert extends Locatable, @static_assert {
   override string toString() { result = "static_assert(..., \"" + getMessage() + "\")" }

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

@@ -363,20 +363,8 @@ class File extends Container, @file {
  */
 class HeaderFile extends File {
   HeaderFile() {
-    exists(string ext | ext = this.getExtension().toLowerCase() |
-      ext = "h" or
-      ext = "r" or
-      /*    ---   */ ext = "hpp" or
-      ext = "hxx" or
-      ext = "h++" or
-      ext = "hh" or
-      ext = "hp" or
-      ext = "tcc" or
-      ext = "tpp" or
-      ext = "txx" or
-      ext = "t++"
-      /*    ---         ---    */
-    )
+    this.getExtension().toLowerCase() =
+      ["h", "r", "hpp", "hxx", "h++", "hh", "hp", "tcc", "tpp", "txx", "t++"]
     or
     not exists(this.getExtension()) and
     exists(Include i | i.getIncludedFile() = this)
@@ -406,7 +394,7 @@ class HeaderFile extends File {
  * `File.compiledAsC`.
  */
 class CFile extends File {
-  CFile() { exists(string ext | ext = this.getExtension().toLowerCase() | ext = "c" or ext = "i") }
+  CFile() { this.getExtension().toLowerCase() = ["c", "i"] }
 
   override string getAPrimaryQlClass() { result = "CFile" }
 }
@@ -419,21 +407,10 @@ class CFile extends File {
  */
 class CppFile extends File {
   CppFile() {
-    exists(string ext | ext = this.getExtension().toLowerCase() |
-      /*     ---     */ ext = "cpp" or
-      ext = "cxx" or
-      ext = "c++" or
-      ext = "cc" or
-      ext = "cp" or
-      ext = "icc" or
-      ext = "ipp" or
-      ext = "ixx" or
-      ext = "i++" or
-      ext = "ii"
-      /*  ---    */
-      // Note: .C files are indistinguishable from .c files on some
-      // file systems, so we just treat them as CFile's.
-    )
+    this.getExtension().toLowerCase() =
+      ["cpp", "cxx", "c++", "cc", "cp", "icc", "ipp", "ixx", "i++", "ii"]
+    // Note: .C files are indistinguishable from .c files on some
+    // file systems, so we just treat them as CFile's.
   }
 
   override string getAPrimaryQlClass() { result = "CppFile" }

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

@@ -334,6 +334,18 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
     )
   }
 
+  /**
+   * Gets the class of which this function, called `memberName`, is a member.
+   *
+   * Prefer to use `getDeclaringType()` or `getName()` directly if you do not
+   * need to reason about both.
+   */
+  pragma[nomagic]
+  Class getClassAndName(string memberName) {
+    this.hasName(memberName) and
+    this.getDeclaringType() = result
+  }
+
   /**
    * Implements `ControlFlowNode.getControlFlowScope`. The `Function` is
    * used to represent the exit node of the control flow graph, so it is
@@ -391,20 +403,30 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   /** Holds if this function has a `noexcept` exception specification. */
   predicate isNoExcept() { getADeclarationEntry().isNoExcept() }
 
-  /** Gets a function that overloads this one. */
+  /**
+   * Gets a function that overloads this one.
+   *
+   * Note: if _overrides_ are wanted rather than _overloads_ then
+   * `MemberFunction::getAnOverridingFunction` should be used instead.
+   */
   Function getAnOverload() {
-    result.getName() = getName() and
-    result.getNamespace() = getNamespace() and
-    result != this and
-    // If this function is declared in a class, only consider other
-    // functions from the same class. Conversely, if this function is not
-    // declared in a class, only consider other functions not declared in a
-    // class.
     (
-      if exists(getDeclaringType())
-      then result.getDeclaringType() = getDeclaringType()
-      else not exists(result.getDeclaringType())
+      // If this function is declared in a class, only consider other
+      // functions from the same class.
+      exists(string name, Class declaringType |
+        candGetAnOverloadMember(name, declaringType, this) and
+        candGetAnOverloadMember(name, declaringType, result)
+      )
+      or
+      // Conversely, if this function is not
+      // declared in a class, only consider other functions not declared in a
+      // class.
+      exists(string name, Namespace namespace |
+        candGetAnOverloadNonMember(name, namespace, this) and
+        candGetAnOverloadNonMember(name, namespace, result)
+      )
     ) and
+    result != this and
     // Instantiations and specializations don't participate in overload
     // resolution.
     not (
@@ -445,50 +467,15 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
       // ... and likewise for destructors.
       this.(Destructor).getADestruction().mayBeGloballyImpure()
     else
-      not exists(string name | this.hasGlobalOrStdName(name) |
-        // Unless it's a function that we know is side-effect-free, it may
-        // have side-effects.
-        name = "strcmp" or
-        name = "wcscmp" or
-        name = "_mbscmp" or
-        name = "strlen" or
-        name = "wcslen" or
-        name = "_mbslen" or
-        name = "_mbslen_l" or
-        name = "_mbstrlen" or
-        name = "_mbstrlen_l" or
-        name = "strnlen" or
-        name = "strnlen_s" or
-        name = "wcsnlen" or
-        name = "wcsnlen_s" or
-        name = "_mbsnlen" or
-        name = "_mbsnlen_l" or
-        name = "_mbstrnlen" or
-        name = "_mbstrnlen_l" or
-        name = "strncmp" or
-        name = "wcsncmp" or
-        name = "_mbsncmp" or
-        name = "_mbsncmp_l" or
-        name = "strchr" or
-        name = "memchr" or
-        name = "wmemchr" or
-        name = "memcmp" or
-        name = "wmemcmp" or
-        name = "_memicmp" or
-        name = "_memicmp_l" or
-        name = "feof" or
-        name = "isdigit" or
-        name = "isxdigit" or
-        name = "abs" or
-        name = "fabs" or
-        name = "labs" or
-        name = "floor" or
-        name = "ceil" or
-        name = "atoi" or
-        name = "atol" or
-        name = "atoll" or
-        name = "atof"
-      )
+      // Unless it's a function that we know is side-effect-free, it may
+      // have side-effects.
+      not this.hasGlobalOrStdName([
+          "strcmp", "wcscmp", "_mbscmp", "strlen", "wcslen", "_mbslen", "_mbslen_l", "_mbstrlen",
+          "_mbstrlen_l", "strnlen", "strnlen_s", "wcsnlen", "wcsnlen_s", "_mbsnlen", "_mbsnlen_l",
+          "_mbstrnlen", "_mbstrnlen_l", "strncmp", "wcsncmp", "_mbsncmp", "_mbsncmp_l", "strchr",
+          "memchr", "wmemchr", "memcmp", "wmemcmp", "_memicmp", "_memicmp_l", "feof", "isdigit",
+          "isxdigit", "abs", "fabs", "labs", "floor", "ceil", "atoi", "atol", "atoll", "atof"
+        ])
   }
 
   /**
@@ -497,6 +484,19 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   override AccessHolder getEnclosingAccessHolder() { result = this.getDeclaringType() }
 }
 
+pragma[noinline]
+private predicate candGetAnOverloadMember(string name, Class declaringType, Function f) {
+  f.getName() = name and
+  f.getDeclaringType() = declaringType
+}
+
+pragma[noinline]
+private predicate candGetAnOverloadNonMember(string name, Namespace namespace, Function f) {
+  f.getName() = name and
+  f.getNamespace() = namespace and
+  not exists(f.getDeclaringType())
+}
+
 /**
  * A particular declaration or definition of a C/C++ function. For example the
  * declaration and definition of `MyFunction` in the following code are each a

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

@@ -205,12 +205,21 @@ class Constructor extends MemberFunction {
 /**
  * A function that defines an implicit conversion.
  */
-abstract class ImplicitConversionFunction extends MemberFunction {
+class ImplicitConversionFunction extends MemberFunction {
+  ImplicitConversionFunction() {
+    // ConversionOperator
+    functions(underlyingElement(this), _, 4)
+    or
+    // ConversionConstructor (deprecated)
+    strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
+    not hasSpecifier("explicit")
+  }
+
   /** Gets the type this `ImplicitConversionFunction` takes as input. */
-  abstract Type getSourceType();
+  Type getSourceType() { none() } // overridden in subclasses
 
   /** Gets the type this `ImplicitConversionFunction` converts to. */
-  abstract Type getDestType();
+  Type getDestType() { none() } // overridden in subclasses
 }
 
 /**

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

@@ -7,8 +7,21 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.metrics.MetricNamespace
 
 /**
- * A C++ namespace.
+ * A C++ namespace. For example the (single) namespace `A` in the following
+ * code:
+ * ```
+ * namespace A
+ * {
+ *   // ...
+ * }
  *
+ * // ...
+ *
+ * namespace A
+ * {
+ *   // ...
+ * }
+ * ```
  * Note that namespaces are somewhat nebulous entities, as they do not in
  * general have a single well-defined location in the source code. The
  * related notion of a `NamespaceDeclarationEntry` is rather more concrete,
@@ -96,10 +109,22 @@ class Namespace extends NameQualifyingElement, @namespace {
 }
 
 /**
- * A declaration of (part of) a C++ namespace.
+ * A declaration of (part of) a C++ namespace. This corresponds to a single
+ * `namespace N { ... }` occurrence in the source code. For example the two
+ * mentions of `A` in the following code:
+ * ```
+ * namespace A
+ * {
+ *   // ...
+ * }
  *
- * This corresponds to a single `namespace N { ... }` occurrence in the
- * source code.
+ * // ...
+ *
+ * namespace A
+ * {
+ *   // ...
+ * }
+ * ```
  */
 class NamespaceDeclarationEntry extends Locatable, @namespace_decl {
   /**
@@ -143,8 +168,9 @@ class UsingEntry extends Locatable, @using {
 
 /**
  * A C++ `using` declaration. For example:
- *
- *   `using std::string;`
+ * ```
+ * using std::string;
+ * ```
  */
 class UsingDeclarationEntry extends UsingEntry {
   UsingDeclarationEntry() {
@@ -162,8 +188,9 @@ class UsingDeclarationEntry extends UsingEntry {
 
 /**
  * A C++ `using` directive. For example:
- *
- *   `using namespace std;`
+ * ```
+ * using namespace std;
+ * ```
  */
 class UsingDirectiveEntry extends UsingEntry {
   UsingDirectiveEntry() {

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

@@ -2,9 +2,14 @@ import semmle.code.cpp.Location
 import semmle.code.cpp.Element
 
 /**
- * A C/C++ preprocessor directive.
- *
- * For example: `#ifdef`, `#line`, or `#pragma`.
+ * A C/C++ preprocessor directive. For example each of the following lines of
+ * code contains a `PreprocessorDirective`:
+ * ```
+ * #pragma once
+ * #ifdef MYDEFINE
+ * #include "myfile.h"
+ * #line 1 "source.c"
+ * ```
  */
 class PreprocessorDirective extends Locatable, @preprocdirect {
   override string toString() { result = "Preprocessor directive" }
@@ -98,9 +103,9 @@ class PreprocessorBranchDirective extends PreprocessorDirective, TPreprocessorBr
  * A C/C++ preprocessor branching directive: `#if`, `#ifdef`, `#ifndef`, or
  * `#elif`.
  *
- * A branching directive can have its condition evaluated at compile-time,
- * and as a result, the preprocessor will either take the branch, or not
- * take the branch.
+ * A branching directive has a condition and that condition may be evaluated
+ * at compile-time.  As a result, the preprocessor will either take the
+ * branch, or not take the branch.
  *
  * However, there are also situations in which a branch's condition isn't
  * evaluated.  The obvious case of this is when the directive is contained
@@ -136,8 +141,13 @@ class PreprocessorBranch extends PreprocessorBranchDirective, @ppd_branch {
 }
 
 /**
- * A C/C++ preprocessor `#if` directive.
- *
+ * A C/C++ preprocessor `#if` directive. For example there is a
+ * `PreprocessorIf` on the first line of the following code:
+ * ```
+ * #if defined(MYDEFINE)
+ * // ...
+ * #endif
+ * ```
  * For the related notion of a directive which causes branching (which
  * includes `#if`, plus also `#ifdef`, `#ifndef`, and `#elif`), see
  * `PreprocessorBranch`.
@@ -147,8 +157,13 @@ class PreprocessorIf extends PreprocessorBranch, @ppd_if {
 }
 
 /**
- * A C/C++ preprocessor `#ifdef` directive.
- *
+ * A C/C++ preprocessor `#ifdef` directive. For example there is a
+ * `PreprocessorIfdef` on the first line of the following code:
+ * ```
+ * #ifdef MYDEFINE
+ * // ...
+ * #endif
+ * ```
  * The syntax `#ifdef X` is shorthand for `#if defined(X)`.
  */
 class PreprocessorIfdef extends PreprocessorBranch, @ppd_ifdef {
@@ -158,8 +173,13 @@ class PreprocessorIfdef extends PreprocessorBranch, @ppd_ifdef {
 }
 
 /**
- * A C/C++ preprocessor `#ifndef` directive.
- *
+ * A C/C++ preprocessor `#ifndef` directive. For example there is a
+ * `PreprocessorIfndef` on the first line of the following code:
+ * ```
+ * #ifndef MYDEFINE
+ * // ...
+ * #endif
+ * ```
  * The syntax `#ifndef X` is shorthand for `#if !defined(X)`.
  */
 class PreprocessorIfndef extends PreprocessorBranch, @ppd_ifndef {
@@ -167,42 +187,80 @@ class PreprocessorIfndef extends PreprocessorBranch, @ppd_ifndef {
 }
 
 /**
- * A C/C++ preprocessor `#else` directive.
+ * A C/C++ preprocessor `#else` directive. For example there is a
+ * `PreprocessorElse` on the fifth line of the following code:
+ * ```
+ * #ifdef MYDEFINE1
+ * // ...
+ * #elif MYDEFINE2
+ * // ...
+ * #else
+ * // ...
+ * #endif
+ * ```
  */
 class PreprocessorElse extends PreprocessorBranchDirective, @ppd_else {
   override string toString() { result = "#else" }
 }
 
 /**
- * A C/C++ preprocessor `#elif` directive.
+ * A C/C++ preprocessor `#elif` directive. For example there is a
+ * `PreprocessorElif` on the third line of the following code:
+ * ```
+ * #ifdef MYDEFINE1
+ * // ...
+ * #elif MYDEFINE2
+ * // ...
+ * #else
+ * // ...
+ * #endif
+ * ```
  */
 class PreprocessorElif extends PreprocessorBranch, @ppd_elif {
   override string toString() { result = "#elif " + this.getHead() }
 }
 
 /**
- * A C/C++ preprocessor `#endif` directive.
+ * A C/C++ preprocessor `#endif` directive. For example there is a
+ * `PreprocessorEndif` on the third line of the following code:
+ * ```
+ * #ifdef MYDEFINE
+ * // ...
+ * #endif
+ * ```
  */
 class PreprocessorEndif extends PreprocessorBranchDirective, @ppd_endif {
   override string toString() { result = "#endif" }
 }
 
 /**
- * A C/C++ preprocessor `#warning` directive.
+ * A C/C++ preprocessor `#warning` directive. For example:
+ * ```
+ * #warning "This configuration is not supported."
+ * ```
  */
 class PreprocessorWarning extends PreprocessorDirective, @ppd_warning {
   override string toString() { result = "#warning " + this.getHead() }
 }
 
 /**
- * A C/C++ preprocessor `#error` directive.
+ * A C/C++ preprocessor `#error` directive. For example:
+ * ```
+ * #error "This configuration is not implemented."
+ * ```
  */
 class PreprocessorError extends PreprocessorDirective, @ppd_error {
   override string toString() { result = "#error " + this.getHead() }
 }
 
 /**
- * A C/C++ preprocessor `#undef` directive.
+ * A C/C++ preprocessor `#undef` directive. For example there is a
+ * `PreprocessorUndef` on the second line of the following code:
+ * ```
+ * #ifdef MYMACRO
+ * #undef MYMACRO
+ * #endif
+ * ```
  */
 class PreprocessorUndef extends PreprocessorDirective, @ppd_undef {
   override string toString() { result = "#undef " + this.getHead() }
@@ -214,7 +272,10 @@ class PreprocessorUndef extends PreprocessorDirective, @ppd_undef {
 }
 
 /**
- * A C/C++ preprocessor `#pragma` directive.
+ * A C/C++ preprocessor `#pragma` directive. For example:
+ * ```
+ * #pragma once
+ * ```
  */
 class PreprocessorPragma extends PreprocessorDirective, @ppd_pragma {
   override string toString() {
@@ -223,7 +284,10 @@ class PreprocessorPragma extends PreprocessorDirective, @ppd_pragma {
 }
 
 /**
- * A C/C++ preprocessor `#line` directive.
+ * A C/C++ preprocessor `#line` directive. For example:
+ * ```
+ * #line 1 "source.c"
+ * ```
  */
 class PreprocessorLine extends PreprocessorDirective, @ppd_line {
   override string toString() { result = "#line " + this.getHead() }

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

@@ -60,7 +60,7 @@ private string getTemplateArgumentString(Declaration d, int i) {
 /**
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
-abstract private class DumpDeclaration extends Declaration {
+private class DumpDeclaration extends Declaration {
   DumpDeclaration() { shouldPrintDeclaration(this) }
 
   /**
@@ -304,7 +304,7 @@ private class SpecifiedDumpType extends DerivedDumpType, SpecifiedType {
       basePrefix = getBaseType().(DumpType).getDeclaratorPrefix() and
       if getBaseType().getUnspecifiedType() instanceof RoutineType
       then result = basePrefix
-      else result = basePrefix + " " + getSpecifierString().trim()
+      else result = basePrefix + " " + getSpecifierString()
     )
   }
 
@@ -312,7 +312,7 @@ private class SpecifiedDumpType extends DerivedDumpType, SpecifiedType {
     exists(string baseSuffix |
       baseSuffix = getBaseType().(DumpType).getDeclaratorSuffixBeforeQualifiers() and
       if getBaseType().getUnspecifiedType() instanceof RoutineType
-      then result = baseSuffix + " " + getSpecifierString().trim()
+      then result = baseSuffix + " " + getSpecifierString()
       else result = baseSuffix
     )
   }
@@ -385,7 +385,7 @@ private class DumpFunction extends DumpDeclaration, Function {
 
   private string getACVQualifier() {
     result = getASpecifier().getName() and
-    (result = "const" or result = "volatile")
+    result = ["const", "volatile"]
   }
 
   private string getDeclaratorSuffix() {

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

@@ -34,8 +34,7 @@ private predicate shouldPrintFunction(Function func) {
 bindingset[s]
 private string escapeString(string s) {
   result =
-    s
-        .replaceAll("\\", "\\\\")
+    s.replaceAll("\\", "\\\\")
         .replaceAll("\n", "\\n")
         .replaceAll("\r", "\\r")
         .replaceAll("\t", "\\t")
@@ -91,7 +90,8 @@ private newtype TPrintASTNode =
   TDeclarationEntryNode(DeclStmt stmt, DeclarationEntry entry) {
     // We create a unique node for each pair of (stmt, entry), to avoid having one node with
     // multiple parents due to extractor bug CPP-413.
-    stmt.getADeclarationEntry() = entry
+    stmt.getADeclarationEntry() = entry and
+    shouldPrintFunction(stmt.getEnclosingFunction())
   } or
   TParametersNode(Function func) { shouldPrintFunction(func) } or
   TConstructorInitializersNode(Constructor ctor) {
@@ -234,11 +234,27 @@ class PrintASTNode extends TPrintASTNode {
   private Function getEnclosingFunction() { result = getParent*().(FunctionNode).getFunction() }
 }
 
+/**
+ * Class that restricts the elements that we compute `qlClass` for.
+ */
+private class PrintableElement extends Element {
+  PrintableElement() {
+    exists(TASTNode(this))
+    or
+    exists(TDeclarationEntryNode(_, this))
+    or
+    this instanceof Type
+  }
+
+  pragma[noinline]
+  string getAPrimaryQlClass0() { result = getAPrimaryQlClass() }
+}
+
 /**
  * Retrieves the canonical QL class(es) for entity `el`
  */
-private string qlClass(ElementBase el) {
-  result = "[" + concat(el.getAPrimaryQlClass(), ",") + "] "
+private string qlClass(PrintableElement el) {
+  result = "[" + concat(el.getAPrimaryQlClass0(), ",") + "] "
   // Alternative implementation -- do not delete. It is useful for QL class discovery.
   //result = "["+ concat(el.getAQlClass(), ",") + "] "
 }

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

@@ -171,8 +171,11 @@ class StdAttribute extends Attribute, @stdattribute {
 }
 
 /**
- * An attribute introduced by Microsoft's `__declspec(name)` syntax, for
- * example: `__declspec(dllimport)`.
+ * An attribute introduced by Microsoft's `__declspec(name)` syntax.  For
+ * example the attribute on the following declaration:
+ * ```
+ * __declspec(dllimport) void myFunction();
+ * ```
  */
 class Declspec extends Attribute, @declspec { }
 
@@ -186,8 +189,13 @@ class MicrosoftAttribute extends Attribute, @msattribute {
 }
 
 /**
- * A C++11 `alignas` construct.
- *
+ * A C++11 `alignas` construct. For example the attribute in the following
+ * code:
+ * ```
+ * struct alignas(16) MyStruct {
+ *   int x;
+ * };
+ * ```
  * Though it doesn't use the attribute syntax, `alignas(...)` is presented
  * as an `Attribute` for consistency with the `[[align(...)]]` attribute.
  */
@@ -197,7 +205,11 @@ class AlignAs extends Attribute, @alignas {
 
 /**
  * A GNU `format` attribute of the form `__attribute__((format(archetype, format-index, first-arg)))`
- * that declares a function to accept a `printf` style format string.
+ * that declares a function to accept a `printf` style format string.  For example the attribute
+ * on the following declaration:
+ * ```
+ * int myPrintf(const char *format, ...) __attribute__((format(printf, 1, 2)));
+ * ```
  */
 class FormatAttribute extends GnuAttribute {
   FormatAttribute() { getName() = "format" }
@@ -242,7 +254,11 @@ class FormatAttribute extends GnuAttribute {
 }
 
 /**
- * An argument to an `Attribute`.
+ * An argument to an `Attribute`. For example the argument "dllimport" on the
+ * attribute in the following code:
+ * ```
+ * __declspec(dllimport) void myFunction();
+ * ```
  */
 class AttributeArgument extends Element, @attribute_arg {
   /**

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

@@ -274,7 +274,7 @@ class Type extends Locatable, @type {
 
   /**
    * Gets this type with any typedefs resolved. For example, given
-   * `typedef C T`, this would resolve `const T&` to `const C&`.
+   * `typedef C T`, this would resolve `const T&` to `const C&`.
    * Note that this will only work if the resolved type actually appears
    * on its own elsewhere in the program.
    */
@@ -577,7 +577,9 @@ class BoolType extends IntegralType {
  * unsigned char e, f;
  * ```
  */
-abstract class CharType extends IntegralType { }
+class CharType extends IntegralType {
+  CharType() { builtintypes(underlyingElement(this), _, [5, 6, 7], _, _, _) }
+}
 
 /**
  * The C/C++ `char` type (which is distinct from `signed char` and
@@ -1304,14 +1306,16 @@ class SpecifiedType extends DerivedType {
   }
 
   /**
+   * INTERNAL: Do not use.
+   *
    * Gets all the specifiers of this type as a string in a fixed order (the order
    * only depends on the specifiers, not on the source program). This is intended
    * for debugging queries only and is an expensive operation.
    */
-  string getSpecifierString() { internalSpecString(this, result, 1) }
+  string getSpecifierString() { result = concat(this.getASpecifier().getName(), " ") }
 
   override string explain() {
-    result = this.getSpecifierString() + "{" + this.getBaseType().explain() + "}"
+    result = this.getSpecifierString() + " {" + this.getBaseType().explain() + "}"
   }
 
   override predicate isDeeplyConst() {
@@ -1540,9 +1544,9 @@ class FunctionPointerIshType extends DerivedType {
 /**
  * A C++ pointer to data member. See 15.5.
  * ```
- * class C { int m; };
+ * class C { public: int m; };
  * int C::* p = &C::m;          // pointer to data member m of class C
- * class C *;
+ * class C c;
  * int val = c.*p;              // access data member
  * ```
  */
@@ -1710,28 +1714,6 @@ class AutoType extends TemplateParameter {
   }
 }
 
-//
-// Internal implementation predicates
-//
-private predicate allSpecifiers(int i, string s) { s = rank[i](string t | specifiers(_, t) | t) }
-
-private predicate internalSpecString(Type t, string res, int i) {
-  (
-    if allSpecifiers(i, t.getASpecifier().getName())
-    then
-      exists(string spec, string rest |
-        allSpecifiers(i, spec) and
-        res = spec + " " + rest and
-        internalSpecString(t, rest, i + 1)
-      )
-    else (
-      allSpecifiers(i, _) and internalSpecString(t, res, i + 1)
-    )
-  )
-  or
-  i = count(Specifier s) + 1 and res = ""
-}
-
 private predicate suppressUnusedThis(Type t) { any() }
 
 /** A source code location referring to a type */

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

@@ -4,8 +4,11 @@
 
 import semmle.files.FileSystem
 
+private class TXMLLocatable =
+  @xmldtd or @xmlelement or @xmlattribute or @xmlnamespace or @xmlcomment or @xmlcharacters;
+
 /** An XML element that has a location. */
-abstract class XMLLocatable extends @xmllocatable {
+class XMLLocatable extends @xmllocatable, TXMLLocatable {
   /** Gets the source location for this element. */
   Location getLocation() { xmllocations(this, result) }
 
@@ -33,7 +36,7 @@ abstract class XMLLocatable extends @xmllocatable {
   }
 
   /** Gets a textual representation of this element. */
-  abstract string toString();
+  string toString() { none() } // overridden in subclasses
 }
 
 /**
@@ -51,7 +54,7 @@ class XMLParent extends @xmlparent {
    * Gets a printable representation of this XML parent.
    * (Intended to be overridden in subclasses.)
    */
-  abstract string getName();
+  string getName() { none() } // overridden in subclasses
 
   /** Gets the file to which this XML parent belongs. */
   XMLFile getFile() { result = this or xmlElements(this, _, _, _, result) }

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

@@ -14,11 +14,7 @@ class PackedTimeType extends Type {
   }
 }
 
-private predicate timeType(string typeName) {
-  typeName = "_SYSTEMTIME" or
-  typeName = "SYSTEMTIME" or
-  typeName = "tm"
-}
+private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm"] }
 
 /**
  * A type that is used to represent times and dates in an 'unpacked' form, that is,

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

@@ -6,7 +6,6 @@ import semmle.code.cpp.Type
 import semmle.code.cpp.commons.CommonType
 import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
-import semmle.code.cpp.models.implementations.Printf
 
 class PrintfFormatAttribute extends FormatAttribute {
   PrintfFormatAttribute() { getArchetype() = ["printf", "__printf__"] }
@@ -35,66 +34,90 @@ class AttributeFormattingFunction extends FormattingFunction {
 
 /**
  * A standard function such as `vprintf` that has a format parameter
- * and a variable argument list of type `va_arg`.
+ * and a variable argument list of type `va_arg`. `formatParamIndex` indicates
+ * the format parameter and `type` indicates the type of `vprintf`:
+ *  - `""` is a `vprintf` variant, `outputParamIndex` is `-1`.
+ *  - `"f"` is a `vfprintf` variant, `outputParamIndex` indicates the output stream parameter.
+ *  - `"s"` is a `vsprintf` variant, `outputParamIndex` indicates the output buffer parameter.
+ *  - `"?"` if the type cannot be deteremined.  `outputParamIndex` is `-1`.
  */
-predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
-  f.getName().regexpMatch("_?_?va?[fs]?n?w?printf(_s)?(_p)?(_l)?") and
+predicate primitiveVariadicFormatter(
+  TopLevelFunction f, string type, int formatParamIndex, int outputParamIndex
+) {
+  type = f.getName().regexpCapture("_?_?va?([fs]?)n?w?printf(_s)?(_p)?(_l)?", 1) and
   (
     if f.getName().matches("%\\_l")
     then formatParamIndex = f.getNumberOfParameters() - 3
     else formatParamIndex = f.getNumberOfParameters() - 2
+  ) and
+  (
+    if type = "" then outputParamIndex = -1 else outputParamIndex = 0 // Conveniently, these buffer parameters are all at index 0.
+  ) and
+  not exists(f.getBlock()) // exclude functions with an implementation in the snapshot as they may not be standard implementations.
+}
+
+private predicate callsVariadicFormatter(
+  Function f, string type, int formatParamIndex, int outputParamIndex
+) {
+  // calls a variadic formatter with `formatParamIndex`, `outputParamIndex` linked
+  exists(FunctionCall fc, int format, int output |
+    variadicFormatter(fc.getTarget(), type, format, output) and
+    fc.getEnclosingFunction() = f and
+    fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
+    fc.getArgument(output) = f.getParameter(outputParamIndex).getAnAccess()
+  )
+  or
+  // calls a variadic formatter with only `formatParamIndex` linked
+  exists(FunctionCall fc, string calledType, int format, int output |
+    variadicFormatter(fc.getTarget(), calledType, format, output) and
+    fc.getEnclosingFunction() = f and
+    fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
+    not fc.getArgument(output) = f.getParameter(_).getAnAccess() and
+    (
+      calledType = "" and
+      type = ""
+      or
+      calledType != "" and
+      type = "?" // we probably should have an `outputParamIndex` link but have lost it.
+    ) and
+    outputParamIndex = -1
   )
 }
 
 /**
- * A standard function such as `vsprintf` that has an output parameter
- * and a variable argument list of type `va_arg`.
+ * Holds if `f` is a function such as `vprintf` that has a format parameter
+ * and a variable argument list of type `va_arg`. `formatParamIndex` indicates
+ * the format parameter and `type` indicates the type of `vprintf`:
+ *  - `""` is a `vprintf` variant, `outputParamIndex` is `-1`.
+ *  - `"f"` is a `vfprintf` variant, `outputParamIndex` indicates the output stream parameter.
+ *  - `"s"` is a `vsprintf` variant, `outputParamIndex` indicates the output buffer parameter.
+ *  - `"?"` if the type cannot be deteremined.  `outputParamIndex` is `-1`.
  */
-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
-    fc.getEnclosingFunction() = f and
-    fc.getArgument(i) = f.getParameter(formatParamIndex).getAnAccess()
-  )
-}
-
-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)
+predicate variadicFormatter(Function f, string type, int formatParamIndex, int outputParamIndex) {
+  primitiveVariadicFormatter(f, type, formatParamIndex, outputParamIndex)
   or
   not f.isVarargs() and
-  callsVariadicFormatterOutput(f, outputParamIndex)
+  callsVariadicFormatter(f, type, formatParamIndex, outputParamIndex)
+}
+
+/**
+ * A standard function such as `vprintf` that has a format parameter
+ * and a variable argument list of type `va_arg`.
+ *
+ * DEPRECATED: Use the four argument version instead.
+ */
+deprecated predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
+  primitiveVariadicFormatter(f, _, formatParamIndex, _)
 }
 
 /**
  * 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`.
+ *
+ * DEPRECATED: Use the four argument version instead.
  */
-predicate variadicFormatter(Function f, int formatParamIndex) {
-  primitiveVariadicFormatter(f, formatParamIndex)
-  or
-  not f.isVarargs() and
-  callsVariadicFormatter(f, formatParamIndex)
+deprecated predicate variadicFormatter(Function f, int formatParamIndex) {
+  variadicFormatter(f, _, formatParamIndex, _)
 }
 
 /**
@@ -104,11 +127,17 @@ predicate variadicFormatter(Function f, int formatParamIndex) {
 class UserDefinedFormattingFunction extends FormattingFunction {
   override string getAPrimaryQlClass() { result = "UserDefinedFormattingFunction" }
 
-  UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _) }
+  UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _, _, _) }
 
-  override int getFormatParameterIndex() { callsVariadicFormatter(this, result) }
+  override int getFormatParameterIndex() { callsVariadicFormatter(this, _, result, _) }
 
-  override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
+  override int getOutputParameterIndex(boolean isStream) {
+    callsVariadicFormatter(this, "f", _, result) and isStream = true
+    or
+    callsVariadicFormatter(this, "s", _, result) and isStream = false
+  }
+
+  override predicate isOutputGlobal() { callsVariadicFormatter(this, "", _, _) }
 }
 
 /**
@@ -874,6 +903,7 @@ class FormatLiteral extends Literal {
    */
   int getNumArgNeeded(int n) {
     exists(this.getConvSpecOffset(n)) and
+    exists(this.getConversionChar(n)) and
     result = count(int mode | hasFormatArgumentIndexFor(n, mode))
   }
 
@@ -1090,8 +1120,7 @@ class FormatLiteral extends Literal {
     then result = this.getFormat().substring(0, this.getConvSpecOffset(0))
     else
       result =
-        this
-            .getFormat()
+        this.getFormat()
             .substring(this.getConvSpecOffset(n - 1) + this.getConvSpec(n - 1).length(),
               this.getConvSpecOffset(n))
   }
@@ -1107,8 +1136,7 @@ class FormatLiteral extends Literal {
         if n > 0
         then
           result =
-            this
-                .getFormat()
+            this.getFormat()
                 .substring(this.getConvSpecOffset(n - 1) + this.getConvSpec(n - 1).length(),
                   this.getFormat().length())
         else result = this.getFormat()

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

@@ -92,7 +92,9 @@ class Snscanf extends ScanfFunction {
     this instanceof TopLevelFunction and
     (
       hasName("_snscanf") or // _snscanf(src, max_amount, format, args...)
-      hasName("_snwscanf") // _snwscanf(src, max_amount, format, args...)
+      hasName("_snwscanf") or // _snwscanf(src, max_amount, format, args...)
+      hasName("_snscanf_l") or // _snscanf_l(src, max_amount, format, locale, args...)
+      hasName("_snwscanf_l") // _snwscanf_l(src, max_amount, format, locale, args...)
       // note that the max_amount is not a limit on the output length, it's an input length
       // limit used with non null-terminated strings.
     )
@@ -101,6 +103,12 @@ class Snscanf extends ScanfFunction {
   override int getInputParameterIndex() { result = 0 }
 
   override int getFormatParameterIndex() { result = 2 }
+
+  /**
+   * Gets the position at which the maximum number of characters in the
+   * input string is specified.
+   */
+  int getInputLengthParameterIndex() { result = 1 }
 }
 
 /**

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

@@ -87,7 +87,7 @@ abstract class MutexType extends Type {
 private Function mustlockCandidate() {
   exists(string name | name = result.getName() |
     name = "lock" or
-    name.suffix(name.length() - 10) = "mutex_lock"
+    name.matches("%mutex\\_lock")
   )
 }
 
@@ -97,7 +97,7 @@ private Function mustlockCandidate() {
 private Function trylockCandidate() {
   exists(string name | name = result.getName() |
     name = "try_lock" or
-    name.suffix(name.length() - 13) = "mutex_trylock"
+    name.matches("%mutex\\_trylock")
   )
 }
 
@@ -107,7 +107,7 @@ private Function trylockCandidate() {
 private Function unlockCandidate() {
   exists(string name | name = result.getName() |
     name = "unlock" or
-    name.suffix(name.length() - 12) = "mutex_unlock"
+    name.matches("%mutex\\_unlock")
   )
 }
 

cpp/ql/src/semmle/code/cpp/controlflow/Dataflow.qll

@@ -13,7 +13,7 @@ import Dereferenced
  * predicates that implement this analysis.
  */
 abstract class DataflowAnnotation extends string {
-  DataflowAnnotation() { this = "pointer-null" or this = "pointer-valid" }
+  DataflowAnnotation() { this = ["pointer-null", "pointer-valid"] }
 
   /** Holds if this annotation is the default annotation. */
   abstract predicate isDefault();
@@ -98,7 +98,7 @@ abstract class DataflowAnnotation extends string {
  * respectively.
  */
 class NullnessAnnotation extends DataflowAnnotation {
-  NullnessAnnotation() { this = "pointer-null" or this = "pointer-valid" }
+  NullnessAnnotation() { this = ["pointer-null", "pointer-valid"] }
 
   override predicate isDefault() { this = "pointer-valid" }
 

cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll

@@ -212,14 +212,9 @@ private predicate addressMayEscapeAt(Expr e) {
 
 private predicate addressMayEscapeMutablyAt(Expr e) {
   addressMayEscapeAt(e) and
-  exists(Type t | t = e.getType().getUnderlyingType() |
-    exists(PointerType pt |
-      pt = t
-      or
-      pt = t.(SpecifiedType).getBaseType()
-    |
-      not pt.getBaseType().isConst()
-    )
+  exists(Type t | t = e.getType().stripTopLevelSpecifiers() |
+    t instanceof PointerType and
+    not t.(PointerType).getBaseType().isConst()
     or
     t instanceof ReferenceType and
     not t.(ReferenceType).getBaseType().isConst()
@@ -227,6 +222,15 @@ private predicate addressMayEscapeMutablyAt(Expr e) {
     // If the address has been cast to an integral type, conservatively assume that it may eventually be cast back to a
     // pointer to non-const type.
     t instanceof IntegralType
+    or
+    // If we go through a temporary object step, we can take a reference to a temporary const pointer
+    // object, where the pointer doesn't point to a const value
+    exists(TemporaryObjectExpr temp, PointerType pt |
+      temp.getConversion() = e.(ReferenceToExpr) and
+      pt = temp.getType().stripTopLevelSpecifiers()
+    |
+      not pt.getBaseType().isConst()
+    )
   )
 }
 
@@ -254,7 +258,7 @@ private predicate addressFromVariableAccess(VariableAccess va, Expr e) {
   // `e` could be a pointer that is converted to a reference as the final step,
   // meaning that we pass a value that is two dereferences away from referring
   // to `va`. This happens, for example, with `void std::vector::push_back(T&&
-  // value);` when called as `v.push_back(&x)`, for a static variable `x`. It
+  // value);` when called as `v.push_back(&x)`, for a variable `x`. It
   // can also happen when taking a reference to a const pointer to a
   // (potentially non-const) value.
   exists(Expr pointerValue |