make WhitespaceContradictsPrecedence silly

Python: Model sources from stdlib HTTP servers

.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
+    

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-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,7 @@
 + 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
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

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-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,35 @@ 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"
+        ]
     )
   }
 }
@@ -475,12 +457,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-191/UnsignedDifferenceExpressionComparedZero.c

@@ -0,0 +1,11 @@
+unsigned long sizeArray;
+
+// BAD: let's consider several values, taking ULONG_MAX =18446744073709551615
+// sizeArray = 60; (sizeArray - 10) = 50; true
+// sizeArray = 10; (sizeArray - 10) = 0; false
+// sizeArray = 1; (sizeArray - 10) = 18446744073709551607; true
+// sizeArray = 0; (sizeArray - 10) = 18446744073709551606; true
+if (sizeArray - 10 > 0)
+
+// GOOD: Prevent overflow by checking the input
+if (sizeArray > 10)

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

@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The code compares the unsigned difference with zero. 
+It is highly probable that the condition is wrong if the difference expression has the unsigned type.  
+The condition holds in all the cases when difference is not equal to zero. 
+It means that we may use condition not equal. But the programmer probably wanted to compare the difference of elements.</p>
+
+<p>False positives include code in which the first difference element is always greater than or equal to the second. 
+For comparison, ">" such conditions are equivalent to "! =", And are recommended for replacement. 
+For comparison "> =", the conditions are always true and are recommended to be excluded.</p>
+
+</overview>
+<recommendation>
+
+<p>Use a simple comparison of two elements, instead of comparing their difference to zero.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of comparison.</p>
+<sample src="UnsignedDifferenceExpressionComparedZero.c" />
+
+</example>
+<references>
+
+<li>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/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Unsigned difference expression compared to zero
+ * @description It is highly probable that the condition is wrong if the difference expression has the unsigned type.
+ *              The condition holds in all the cases when difference is not equal to zero. It means that we may use condition not equal.
+ *              But the programmer probably wanted to compare the difference of elements.
+ * @kind problem
+ * @id cpp/unsigned-difference-expression-compared-zero
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       external/cwe/cwe-191
+ */
+
+import cpp
+import semmle.code.cpp.commons.Exclusions
+
+from RelationalOperation ro, SubExpr sub
+where
+  not isFromMacroDefinition(ro) and
+  ro.getLesserOperand().getValue().toInt() = 0 and
+  ro.getGreaterOperand() = sub and
+  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
+select ro, "Difference in condition is always greater than or equal to zero"

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/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() }
 }
 
 /**

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

@@ -445,50 +445,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"
+        ])
   }
 
   /**

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/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/Type.qll

@@ -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() {
@@ -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,87 @@ 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.
+}
+
+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 +124,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, "", _, _) }
 }
 
 /**
@@ -1090,8 +1116,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 +1132,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/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 |

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll

@@ -802,14 +802,9 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract boolean toBoolNonEmpty();
 
-  predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) }
+  TypedContent getHead() { this = TFrontHead(result) }
 
-  predicate isClearedAt(Node n) {
-    exists(TypedContent tc |
-      this.headUsesContent(tc) and
-      clearsContent(n, tc.getContent())
-    )
-  }
+  predicate isClearedAt(Node n) { clearsContent(n, getHead().getContent()) }
 }
 
 class AccessPathFrontNil extends AccessPathFront, TFrontNil {

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll

@@ -280,6 +280,15 @@ predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub impl
 
 int accessPathLimit() { result = 5 }
 
+/** The unit type. */
+private newtype TUnit = TMkUnit()
+
+/** The trivial type with a single element. */
+class Unit extends TUnit {
+  /** Gets a textual representation of this element. */
+  string toString() { result = "unit" }
+}
+
 /**
  * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
  * modified or its modification cannot be observed, for example if it is a

cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll

@@ -620,7 +620,8 @@ module FlowVar_internal {
   private predicate largeVariable(Variable v, int liveBlocks, int defs) {
     liveBlocks = strictcount(SubBasicBlock sbb | variableLiveInSBB(sbb, v)) and
     defs = strictcount(SubBasicBlock sbb | exists(TBlockVar(sbb, v))) and
-    liveBlocks * defs > 1000000
+    // Convert to float to avoid int overflow (32-bit two's complement)
+    liveBlocks.(float) * defs.(float) > 100000.0
   }
 
   /**

cpp/ql/src/semmle/code/cpp/exprs/ComparisonOperation.qll

@@ -64,7 +64,7 @@ class RelationalOperation extends ComparisonOperation, @rel_op_expr {
    * if the overall expression evaluates to `true`; for example on
    * `x <= 20` this is the `20`, and on `y > 0` it is `y`.
    */
-  abstract Expr getGreaterOperand();
+  Expr getGreaterOperand() { none() } // overridden in subclasses
 
   /**
    * Gets the operand on the "lesser" (or "lesser-or-equal") side
@@ -72,7 +72,7 @@ class RelationalOperation extends ComparisonOperation, @rel_op_expr {
    * if the overall expression evaluates to `true`; for example on
    * `x <= 20` this is `x`, and on `y > 0` it is the `0`.
    */
-  abstract Expr getLesserOperand();
+  Expr getLesserOperand() { none() } // overridden in subclasses
 }
 
 /**

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -6,7 +6,6 @@ import semmle.code.cpp.Element
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 private import semmle.code.cpp.internal.AddressConstantExpression
-private import semmle.code.cpp.models.implementations.Allocation
 
 /**
  * A C/C++ expression.
@@ -839,7 +838,7 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
    * For example, for `new int` the result is `int`.
    * For `new int[5]` the result is `int[5]`.
    */
-  abstract Type getAllocatedType();
+  Type getAllocatedType() { none() } // overridden in subclasses
 
   /**
    * Gets the pointer `p` if this expression is of the form `new(p) T...`.
@@ -848,8 +847,7 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
    */
   Expr getPlacementPointer() {
     result =
-      this
-          .getAllocatorCall()
+      this.getAllocatorCall()
           .getArgument(this.getAllocator().(OperatorNewAllocationFunction).getPlacementArgument())
   }
 }

cpp/ql/src/semmle/code/cpp/exprs/Literal.qll

@@ -47,7 +47,17 @@ class LabelLiteral extends Literal {
 }
 
 /** A character literal or a string literal. */
-abstract class TextLiteral extends Literal {
+class TextLiteral extends Literal {
+  TextLiteral() {
+    // String Literal
+    // Note that `AggregateLiteral`s can also have an array type, but they derive from
+    // @aggregateliteral rather than @literal.
+    this.getType() instanceof ArrayType
+    or
+    // Char literal
+    this.getValueText().regexpMatch("(?s)\\s*L?'.*")
+  }
+
   /** Gets a hex escape sequence that appears in the character or string literal (see [lex.ccon] in the C++ Standard). */
   string getAHexEscapeSequence(int occurrence, int offset) {
     result = getValueText().regexpFind("(?<!\\\\)\\\\x[0-9a-fA-F]+", occurrence, offset)

cpp/ql/src/semmle/code/cpp/exprs/LogicalOperation.qll

@@ -39,7 +39,7 @@ class BinaryLogicalOperation extends BinaryOperation, @bin_log_op_expr {
    * is true, `x` and `y` must also be true, so `impliesValue(x, true, true)` and
    * `impliesValue(y, true, true)` hold.
    */
-  abstract predicate impliesValue(Expr part, boolean partIsTrue, boolean wholeIsTrue);
+  predicate impliesValue(Expr part, boolean partIsTrue, boolean wholeIsTrue) { none() } // overridden in subclasses
 }
 
 /**

cpp/ql/src/semmle/code/cpp/headers/MultipleInclusion.qll

@@ -160,8 +160,7 @@ predicate ifndefDirective(PreprocessorDirective ppd, string macro) {
   ppd instanceof PreprocessorIf and
   exists(string head | head = ppd.getHead() |
     macro =
-      head
-          .replaceAll("(", " ")
+      head.replaceAll("(", " ")
           .replaceAll(")", "")
           .replaceAll("\t", " ")
           .regexpCapture("[ ]*![ ]*defined[ ]+([^ ]*)[ ]*", 1)

cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll

@@ -59,7 +59,7 @@ class Namespace extends @namespace {
   }
 }
 
-abstract class Declaration extends @declaration {
+class Declaration extends @declaration {
   string toString() { result = "QualifiedName Declaration" }
 
   /** Gets the name of this declaration. */

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -36,24 +36,12 @@ private predicate predictableInstruction(Instruction instr) {
  * library's `returnArgument` predicate.
  */
 predicate predictableOnlyFlow(string name) {
-  name = "strcasestr" or
-  name = "strchnul" or
-  name = "strchr" or
-  name = "strchrnul" or
-  name = "strcmp" or
-  name = "strcspn" or
-  name = "strncmp" or
-  name = "strndup" or
-  name = "strnlen" or
-  name = "strrchr" or
-  name = "strspn" or
-  name = "strstr" or
-  name = "strtod" or
-  name = "strtof" or
-  name = "strtol" or
-  name = "strtoll" or
-  name = "strtoq" or
-  name = "strtoul"
+  name =
+    [
+      "strcasestr", "strchnul", "strchr", "strchrnul", "strcmp", "strcspn", "strncmp", "strndup",
+      "strnlen", "strrchr", "strspn", "strstr", "strtod", "strtof", "strtol", "strtoll", "strtoq",
+      "strtoul"
+    ]
 }
 
 private DataFlow::Node getNodeForSource(Expr source) {
@@ -652,8 +640,7 @@ module TaintedWithPath {
     override predicate hasLocationInfo(
       string filepath, int startline, int startcolumn, int endline, int endcolumn
     ) {
-      this
-          .inner()
+      this.inner()
           .getLocation()
           .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
     }

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

@@ -802,14 +802,9 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract boolean toBoolNonEmpty();
 
-  predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) }
+  TypedContent getHead() { this = TFrontHead(result) }
 
-  predicate isClearedAt(Node n) {
-    exists(TypedContent tc |
-      this.headUsesContent(tc) and
-      clearsContent(n, tc.getContent())
-    )
-  }
+  predicate isClearedAt(Node n) { clearsContent(n, getHead().getContent()) }
 }
 
 class AccessPathFrontNil extends AccessPathFront, TFrontNil {

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

@@ -28,11 +28,7 @@ private class PrimaryArgumentNode extends ArgumentNode {
 
   PrimaryArgumentNode() { exists(CallInstruction call | op = call.getAnArgumentOperand()) }
 
-  override predicate argumentOf(DataFlowCall call, int pos) {
-    op = call.getPositionalArgumentOperand(pos)
-    or
-    op = call.getThisArgumentOperand() and pos = -1
-  }
+  override predicate argumentOf(DataFlowCall call, int pos) { op = call.getArgumentOperand(pos) }
 
   override string toString() {
     result = "Argument " + op.(PositionalArgumentOperand).getIndex()
@@ -110,10 +106,10 @@ class ReturnIndirectionNode extends ReturnNode {
   override ReturnIndirectionInstruction primary;
 
   override ReturnKind getKind() {
-    result = TIndirectReturnKind(-1) and
-    primary.isThisIndirection()
-    or
-    result = TIndirectReturnKind(primary.getParameter().getIndex())
+    exists(int index |
+      primary.hasIndex(index) and
+      result = TIndirectReturnKind(index)
+    )
   }
 }
 
@@ -500,13 +496,6 @@ class DataFlowType = IRType;
 
 /** A function call relevant for data flow. */
 class DataFlowCall extends CallInstruction {
-  /**
-   * Gets the nth argument for this call.
-   *
-   * The range of `n` is from `0` to `getNumberOfArguments() - 1`.
-   */
-  Node getArgument(int n) { result.asInstruction() = this.getPositionalArgument(n) }
-
   Function getEnclosingCallable() { result = this.getEnclosingFunction() }
 }
 
@@ -514,6 +503,15 @@ predicate isUnreachableInCall(Node n, DataFlowCall call) { none() } // stub impl
 
 int accessPathLimit() { result = 5 }
 
+/** The unit type. */
+private newtype TUnit = TMkUnit()
+
+/** The trivial type with a single element. */
+class Unit extends TUnit {
+  /** Gets a textual representation of this element. */
+  string toString() { result = "unit" }
+}
+
 /**
  * Holds if `n` does not require a `PostUpdateNode` as it either cannot be
  * modified or its modification cannot be observed, for example if it is a

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

@@ -95,7 +95,7 @@ class Node extends TIRDataFlowNode {
    * Gets the uninitialized local variable corresponding to this node, if
    * any.
    */
-  LocalVariable asUninitialized() { none() }
+  deprecated LocalVariable asUninitialized() { none() }
 
   /**
    * Gets an upper bound on the type of this node.
@@ -266,10 +266,8 @@ class ParameterIndirectionNode extends ParameterNode {
 
   override predicate isParameterOf(Function f, int pos) {
     exists(int index |
-      f.getParameter(index) = instr.getParameter()
-      or
-      index = -1 and
-      instr.getIRVariable().(IRThisVariable).getEnclosingFunction() = f
+      instr.getEnclosingFunction() = f and
+      instr.hasIndex(index)
     |
       pos = getArgumentPosOfSideEffect(index)
     )
@@ -476,16 +474,8 @@ class DefinitionByReferenceNode extends InstructionNode {
       instr
           .getPrimaryInstruction()
           .(CallInstruction)
-          .getPositionalArgument(instr.getIndex())
+          .getArgument(instr.getIndex())
           .getUnconvertedResultExpression()
-    or
-    result =
-      instr
-          .getPrimaryInstruction()
-          .(CallInstruction)
-          .getThisArgument()
-          .getUnconvertedResultExpression() and
-    instr.getIndex() = -1
   }
 
   /** Gets the parameter through which this value is assigned. */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll

@@ -163,6 +163,46 @@ class IRBlock extends IRBlockBase {
     not strictlyDominates(result)
   }
 
+  /**
+   * Holds if this block immediately post-dominates `block`.
+   *
+   * Block `A` immediate post-dominates block `B` if block `A` strictly post-dominates block `B` and
+   * block `B` is a direct successor of block `A`.
+   */
+  final predicate immediatelyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates(this, block)
+  }
+
+  /**
+   * Holds if this block strictly post-dominates `block`.
+   *
+   * Block `A` strictly post-dominates block `B` if block `A` post-dominates block `B` and blocks `A`
+   * and `B` are not the same block.
+   */
+  final predicate strictlyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates+(this, block)
+  }
+
+  /**
+   * Holds if this block is a post-dominator of `block`.
+   *
+   * Block `A` post-dominates block `B` if any control flow path from `B` to the exit block of the
+   * function must pass through block `A`. A block always post-dominates itself.
+   */
+  final predicate postDominates(IRBlock block) { strictlyPostDominates(block) or this = block }
+
+  /**
+   * Gets a block on the post-dominance frontier of this block.
+   *
+   * The post-dominance frontier of block `A` is the set of blocks `B` such that block `A` does not
+   * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
+   */
+  pragma[noinline]
+  final IRBlock postPominanceFrontier() {
+    postDominates(result.getASuccessor()) and
+    not strictlyPostDominates(result)
+  }
+
   /**
    * Holds if this block is reachable from the entry block of its function.
    */
@@ -280,3 +320,12 @@ private module Cached {
 }
 
 private Instruction getFirstInstruction(TIRBlock block) { block = MkIRBlock(result) }
+
+private predicate blockFunctionExit(IRBlock exit) {
+  exit.getLastInstruction() instanceof ExitFunctionInstruction
+}
+
+private predicate blockPredecessor(IRBlock src, IRBlock pred) { src.getAPredecessor() = pred }
+
+private predicate blockImmediatelyPostDominates(IRBlock postDominator, IRBlock block) =
+  idominance(blockFunctionExit/1, blockPredecessor/2)(_, postDominator, block)

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -582,6 +582,17 @@ class InitializeParameterInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the parameter with index `index`, or
+   * if `index` is `-1` and this instruction initializes `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -605,6 +616,18 @@ class InitializeIndirectionInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the memory pointed to by the parameter with
+   * index `index`, or if `index` is `-1` and this instruction initializes the memory
+   * pointed to by `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -779,6 +802,17 @@ class ReturnIndirectionInstruction extends VariableInstruction {
    * Holds if this instruction is the return indirection for `this`.
    */
   final predicate isThisIndirection() { var instanceof IRThisVariable }
+
+  /**
+   * Holds if this instruction is the return indirection for the parameter with index `index`, or
+   * if this instruction is the return indirection for `this` and `index` is `-1`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.isThisIndirection()
+  }
 }
 
 /**
@@ -1591,6 +1625,22 @@ class CallInstruction extends Instruction {
     result = getPositionalArgumentOperand(index).getDef()
   }
 
+  /**
+   * Gets the argument operand at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final ArgumentOperand getArgumentOperand(int index) {
+    index >= 0 and result = getPositionalArgumentOperand(index)
+    or
+    index = -1 and result = getThisArgumentOperand()
+  }
+
+  /**
+   * Gets the argument at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final Instruction getArgument(int index) { result = getArgumentOperand(index).getDef() }
+
   /**
    * Gets the number of arguments of the call, including the `this` pointer, if any.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -562,6 +562,9 @@ private Overlap getVariableMemoryLocationOverlap(
       use.getEndBitOffset())
 }
 
+bindingset[result, b]
+private boolean unbindBool(boolean b) { result != b.booleanNot() }
+
 MemoryLocation getResultMemoryLocation(Instruction instr) {
   exists(MemoryAccessKind kind, boolean isMayAccess |
     kind = instr.getResultMemoryAccess() and
@@ -574,7 +577,8 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
           exists(Allocation var, IRType type, IntValue startBitOffset, IntValue endBitOffset |
             hasResultMemoryAccess(instr, var, type, _, startBitOffset, endBitOffset, isMayAccess) and
             result =
-              TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset, isMayAccess)
+              TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset,
+                unbindBool(isMayAccess))
           )
         else result = TUnknownMemoryLocation(instr.getEnclosingIRFunction(), isMayAccess)
       )
@@ -582,7 +586,7 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
       kind instanceof EntireAllocationMemoryAccess and
       result =
         TEntireAllocationMemoryLocation(getAddressOperandAllocation(instr.getResultAddressOperand()),
-          isMayAccess)
+          unbindBool(isMayAccess))
       or
       kind instanceof EscapedMemoryAccess and
       result = TAllAliasedMemory(instr.getEnclosingIRFunction(), isMayAccess)

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -403,16 +403,27 @@ private import PhiInsertion
  * instruction for the virtual variable as a whole.
  */
 private module PhiInsertion {
+  /**
+   * Holds if `phiBlock` is a block in the dominance frontier of a block that has a definition of the
+   * memory location `defLocation`.
+   */
+  pragma[noinline]
+  private predicate dominanceFrontierOfDefinition(
+    Alias::MemoryLocation defLocation, OldBlock phiBlock
+  ) {
+    exists(OldBlock defBlock |
+      phiBlock = Dominance::getDominanceFrontier(defBlock) and
+      definitionHasDefinitionInBlock(defLocation, defBlock)
+    )
+  }
+
   /**
    * Holds if a `Phi` instruction needs to be inserted for location `defLocation` at the beginning of block `phiBlock`.
    */
   predicate definitionHasPhiNode(Alias::MemoryLocation defLocation, OldBlock phiBlock) {
-    exists(OldBlock defBlock |
-      phiBlock = Dominance::getDominanceFrontier(defBlock) and
-      definitionHasDefinitionInBlock(defLocation, defBlock) and
-      /* We can also eliminate those nodes where the definition is not live on any incoming edge */
-      definitionLiveOnEntryToBlock(defLocation, phiBlock)
-    )
+    dominanceFrontierOfDefinition(defLocation, phiBlock) and
+    /* We can also eliminate those nodes where the definition is not live on any incoming edge */
+    definitionLiveOnEntryToBlock(defLocation, phiBlock)
   }
 
   /**

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRBlock.qll

@@ -163,6 +163,46 @@ class IRBlock extends IRBlockBase {
     not strictlyDominates(result)
   }
 
+  /**
+   * Holds if this block immediately post-dominates `block`.
+   *
+   * Block `A` immediate post-dominates block `B` if block `A` strictly post-dominates block `B` and
+   * block `B` is a direct successor of block `A`.
+   */
+  final predicate immediatelyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates(this, block)
+  }
+
+  /**
+   * Holds if this block strictly post-dominates `block`.
+   *
+   * Block `A` strictly post-dominates block `B` if block `A` post-dominates block `B` and blocks `A`
+   * and `B` are not the same block.
+   */
+  final predicate strictlyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates+(this, block)
+  }
+
+  /**
+   * Holds if this block is a post-dominator of `block`.
+   *
+   * Block `A` post-dominates block `B` if any control flow path from `B` to the exit block of the
+   * function must pass through block `A`. A block always post-dominates itself.
+   */
+  final predicate postDominates(IRBlock block) { strictlyPostDominates(block) or this = block }
+
+  /**
+   * Gets a block on the post-dominance frontier of this block.
+   *
+   * The post-dominance frontier of block `A` is the set of blocks `B` such that block `A` does not
+   * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
+   */
+  pragma[noinline]
+  final IRBlock postPominanceFrontier() {
+    postDominates(result.getASuccessor()) and
+    not strictlyPostDominates(result)
+  }
+
   /**
    * Holds if this block is reachable from the entry block of its function.
    */
@@ -280,3 +320,12 @@ private module Cached {
 }
 
 private Instruction getFirstInstruction(TIRBlock block) { block = MkIRBlock(result) }
+
+private predicate blockFunctionExit(IRBlock exit) {
+  exit.getLastInstruction() instanceof ExitFunctionInstruction
+}
+
+private predicate blockPredecessor(IRBlock src, IRBlock pred) { src.getAPredecessor() = pred }
+
+private predicate blockImmediatelyPostDominates(IRBlock postDominator, IRBlock block) =
+  idominance(blockFunctionExit/1, blockPredecessor/2)(_, postDominator, block)

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -582,6 +582,17 @@ class InitializeParameterInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the parameter with index `index`, or
+   * if `index` is `-1` and this instruction initializes `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -605,6 +616,18 @@ class InitializeIndirectionInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the memory pointed to by the parameter with
+   * index `index`, or if `index` is `-1` and this instruction initializes the memory
+   * pointed to by `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -779,6 +802,17 @@ class ReturnIndirectionInstruction extends VariableInstruction {
    * Holds if this instruction is the return indirection for `this`.
    */
   final predicate isThisIndirection() { var instanceof IRThisVariable }
+
+  /**
+   * Holds if this instruction is the return indirection for the parameter with index `index`, or
+   * if this instruction is the return indirection for `this` and `index` is `-1`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.isThisIndirection()
+  }
 }
 
 /**
@@ -1591,6 +1625,22 @@ class CallInstruction extends Instruction {
     result = getPositionalArgumentOperand(index).getDef()
   }
 
+  /**
+   * Gets the argument operand at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final ArgumentOperand getArgumentOperand(int index) {
+    index >= 0 and result = getPositionalArgumentOperand(index)
+    or
+    index = -1 and result = getThisArgumentOperand()
+  }
+
+  /**
+   * Gets the argument at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final Instruction getArgument(int index) { result = getArgumentOperand(index).getDef() }
+
   /**
    * Gets the number of arguments of the call, including the `this` pointer, if any.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -528,9 +528,9 @@ class TranslatedSideEffect extends TranslatedElement, TTranslatedArgumentSideEff
     tag instanceof OnlyInstructionTag and
     operandTag instanceof BufferSizeOperandTag and
     result =
-      getTranslatedExpr(call
-            .getArgument(call.getTarget().(SideEffectFunction).getParameterSizeIndex(index))
-            .getFullyConverted()).getResult()
+      getTranslatedExpr(call.getArgument(call.getTarget()
+              .(SideEffectFunction)
+              .getParameterSizeIndex(index)).getFullyConverted()).getResult()
   }
 
   override CppType getInstructionMemoryOperandType(InstructionTag tag, TypedOperandTag operandTag) {

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRBlock.qll

@@ -163,6 +163,46 @@ class IRBlock extends IRBlockBase {
     not strictlyDominates(result)
   }
 
+  /**
+   * Holds if this block immediately post-dominates `block`.
+   *
+   * Block `A` immediate post-dominates block `B` if block `A` strictly post-dominates block `B` and
+   * block `B` is a direct successor of block `A`.
+   */
+  final predicate immediatelyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates(this, block)
+  }
+
+  /**
+   * Holds if this block strictly post-dominates `block`.
+   *
+   * Block `A` strictly post-dominates block `B` if block `A` post-dominates block `B` and blocks `A`
+   * and `B` are not the same block.
+   */
+  final predicate strictlyPostDominates(IRBlock block) {
+    blockImmediatelyPostDominates+(this, block)
+  }
+
+  /**
+   * Holds if this block is a post-dominator of `block`.
+   *
+   * Block `A` post-dominates block `B` if any control flow path from `B` to the exit block of the
+   * function must pass through block `A`. A block always post-dominates itself.
+   */
+  final predicate postDominates(IRBlock block) { strictlyPostDominates(block) or this = block }
+
+  /**
+   * Gets a block on the post-dominance frontier of this block.
+   *
+   * The post-dominance frontier of block `A` is the set of blocks `B` such that block `A` does not
+   * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
+   */
+  pragma[noinline]
+  final IRBlock postPominanceFrontier() {
+    postDominates(result.getASuccessor()) and
+    not strictlyPostDominates(result)
+  }
+
   /**
    * Holds if this block is reachable from the entry block of its function.
    */
@@ -280,3 +320,12 @@ private module Cached {
 }
 
 private Instruction getFirstInstruction(TIRBlock block) { block = MkIRBlock(result) }
+
+private predicate blockFunctionExit(IRBlock exit) {
+  exit.getLastInstruction() instanceof ExitFunctionInstruction
+}
+
+private predicate blockPredecessor(IRBlock src, IRBlock pred) { src.getAPredecessor() = pred }
+
+private predicate blockImmediatelyPostDominates(IRBlock postDominator, IRBlock block) =
+  idominance(blockFunctionExit/1, blockPredecessor/2)(_, postDominator, block)

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -582,6 +582,17 @@ class InitializeParameterInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the parameter with index `index`, or
+   * if `index` is `-1` and this instruction initializes `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -605,6 +616,18 @@ class InitializeIndirectionInstruction extends VariableInstruction {
    * Gets the parameter initialized by this instruction.
    */
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  /**
+   * Holds if this instruction initializes the memory pointed to by the parameter with
+   * index `index`, or if `index` is `-1` and this instruction initializes the memory
+   * pointed to by `this`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.getIRVariable() instanceof IRThisVariable
+  }
 }
 
 /**
@@ -779,6 +802,17 @@ class ReturnIndirectionInstruction extends VariableInstruction {
    * Holds if this instruction is the return indirection for `this`.
    */
   final predicate isThisIndirection() { var instanceof IRThisVariable }
+
+  /**
+   * Holds if this instruction is the return indirection for the parameter with index `index`, or
+   * if this instruction is the return indirection for `this` and `index` is `-1`.
+   */
+  pragma[noinline]
+  final predicate hasIndex(int index) {
+    index >= 0 and index = this.getParameter().getIndex()
+    or
+    index = -1 and this.isThisIndirection()
+  }
 }
 
 /**
@@ -1591,6 +1625,22 @@ class CallInstruction extends Instruction {
     result = getPositionalArgumentOperand(index).getDef()
   }
 
+  /**
+   * Gets the argument operand at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final ArgumentOperand getArgumentOperand(int index) {
+    index >= 0 and result = getPositionalArgumentOperand(index)
+    or
+    index = -1 and result = getThisArgumentOperand()
+  }
+
+  /**
+   * Gets the argument at the specified index, or `this` if `index` is `-1`.
+   */
+  pragma[noinline]
+  final Instruction getArgument(int index) { result = getArgumentOperand(index).getDef() }
+
   /**
    * Gets the number of arguments of the call, including the `this` pointer, if any.
    */

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -403,16 +403,27 @@ private import PhiInsertion
  * instruction for the virtual variable as a whole.
  */
 private module PhiInsertion {
+  /**
+   * Holds if `phiBlock` is a block in the dominance frontier of a block that has a definition of the
+   * memory location `defLocation`.
+   */
+  pragma[noinline]
+  private predicate dominanceFrontierOfDefinition(
+    Alias::MemoryLocation defLocation, OldBlock phiBlock
+  ) {
+    exists(OldBlock defBlock |
+      phiBlock = Dominance::getDominanceFrontier(defBlock) and
+      definitionHasDefinitionInBlock(defLocation, defBlock)
+    )
+  }
+
   /**
    * Holds if a `Phi` instruction needs to be inserted for location `defLocation` at the beginning of block `phiBlock`.
    */
   predicate definitionHasPhiNode(Alias::MemoryLocation defLocation, OldBlock phiBlock) {
-    exists(OldBlock defBlock |
-      phiBlock = Dominance::getDominanceFrontier(defBlock) and
-      definitionHasDefinitionInBlock(defLocation, defBlock) and
-      /* We can also eliminate those nodes where the definition is not live on any incoming edge */
-      definitionLiveOnEntryToBlock(defLocation, phiBlock)
-    )
+    dominanceFrontierOfDefinition(defLocation, phiBlock) and
+    /* We can also eliminate those nodes where the definition is not live on any incoming edge */
+    definitionLiveOnEntryToBlock(defLocation, phiBlock)
   }
 
   /**

cpp/ql/src/semmle/code/cpp/models/Models.qll

@@ -1,6 +1,7 @@
 private import implementations.Allocation
 private import implementations.Deallocation
 private import implementations.Fread
+private import implementations.Getenv
 private import implementations.Gets
 private import implementations.IdentityFunction
 private import implementations.Inet

cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll

@@ -10,99 +10,62 @@ import semmle.code.cpp.models.interfaces.Allocation
  * An allocation function (such as `malloc`) that has an argument for the size
  * in bytes.
  */
-class MallocAllocationFunction extends AllocationFunction {
+private class MallocAllocationFunction extends AllocationFunction {
   int sizeArg;
 
   MallocAllocationFunction() {
-    exists(string name |
-      hasGlobalOrStdName(name) and
-      // malloc(size)
-      (name = "malloc" and sizeArg = 0)
-      or
-      hasGlobalName(name) and
-      (
-        // ExAllocatePool(type, size)
-        name = "ExAllocatePool" and sizeArg = 1
-        or
-        // ExAllocatePool(type, size, tag)
-        name = "ExAllocatePoolWithTag" and sizeArg = 1
-        or
-        // ExAllocatePoolWithTagPriority(type, size, tag, priority)
-        name = "ExAllocatePoolWithTagPriority" and sizeArg = 1
-        or
-        // ExAllocatePoolWithQuota(type, size)
-        name = "ExAllocatePoolWithQuota" and sizeArg = 1
-        or
-        // ExAllocatePoolWithQuotaTag(type, size, tag)
-        name = "ExAllocatePoolWithQuotaTag" and sizeArg = 1
-        or
-        // IoAllocateMdl(address, size, flag, flag, irp)
-        name = "IoAllocateMdl" and sizeArg = 1
-        or
-        // IoAllocateErrorLogEntry(object, size)
-        name = "IoAllocateErrorLogEntry" and sizeArg = 1
-        or
-        // MmAllocateContiguousMemory(size, maxaddress)
-        name = "MmAllocateContiguousMemory" and sizeArg = 0
-        or
-        // MmAllocateContiguousNodeMemory(size, minaddress, maxaddress, bound, flag, prefer)
-        name = "MmAllocateContiguousNodeMemory" and sizeArg = 0
-        or
-        // MmAllocateContiguousMemorySpecifyCache(size, minaddress, maxaddress, bound, type)
-        name = "MmAllocateContiguousMemorySpecifyCache" and sizeArg = 0
-        or
-        // MmAllocateContiguousMemorySpecifyCacheNode(size, minaddress, maxaddress, bound, type, prefer)
-        name = "MmAllocateContiguousMemorySpecifyCacheNode" and sizeArg = 0
-        or
-        // MmAllocateNonCachedMemory(size)
-        name = "MmAllocateNonCachedMemory" and sizeArg = 0
-        or
-        // MmAllocateMappingAddress(size, tag)
-        name = "MmAllocateMappingAddress" and sizeArg = 0
-        or
-        // MmAllocatePagesForMdl(minaddress, maxaddress, skip, size)
-        name = "MmAllocatePagesForMdl" and sizeArg = 3
-        or
-        // MmAllocatePagesForMdlEx(minaddress, maxaddress, skip, size, type, flags)
-        name = "MmAllocatePagesForMdlEx" and sizeArg = 3
-        or
-        // MmAllocateNodePagesForMdlEx(minaddress, maxaddress, skip, size, type, prefer, flags)
-        name = "MmAllocateNodePagesForMdlEx" and sizeArg = 3
-        or
-        // LocalAlloc(flags, size)
-        name = "LocalAlloc" and sizeArg = 1
-        or
-        // GlobalAlloc(flags, size)
-        name = "GlobalAlloc" and sizeArg = 1
-        or
-        // HeapAlloc(heap, flags, size)
-        name = "HeapAlloc" and sizeArg = 2
-        or
-        // VirtualAlloc(address, size, type, flag)
-        name = "VirtualAlloc" and sizeArg = 1
-        or
-        // CoTaskMemAlloc(size)
-        name = "CoTaskMemAlloc" and sizeArg = 0
-        or
-        // kmem_alloc(size, flags)
-        name = "kmem_alloc" and sizeArg = 0
-        or
-        // kmem_zalloc(size, flags)
-        name = "kmem_zalloc" and sizeArg = 0
-        or
-        // CRYPTO_malloc(size_t num, const char *file, int line)
-        name = "CRYPTO_malloc" and sizeArg = 0
-        or
-        // CRYPTO_zalloc(size_t num, const char *file, int line)
-        name = "CRYPTO_zalloc" and sizeArg = 0
-        or
-        // CRYPTO_secure_malloc(size_t num, const char *file, int line)
-        name = "CRYPTO_secure_malloc" and sizeArg = 0
-        or
-        // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
-        name = "CRYPTO_secure_zalloc" and sizeArg = 0
-      )
-    )
+    // --- C library allocation
+    hasGlobalOrStdName("malloc") and // malloc(size)
+    sizeArg = 0
+    or
+    hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "MmAllocateContiguousMemory", // MmAllocateContiguousMemory(size, maxaddress)
+        "MmAllocateContiguousNodeMemory", // MmAllocateContiguousNodeMemory(size, minaddress, maxaddress, bound, flag, prefer)
+        "MmAllocateContiguousMemorySpecifyCache", // MmAllocateContiguousMemorySpecifyCache(size, minaddress, maxaddress, bound, type)
+        "MmAllocateContiguousMemorySpecifyCacheNode", // MmAllocateContiguousMemorySpecifyCacheNode(size, minaddress, maxaddress, bound, type, prefer)
+        "MmAllocateNonCachedMemory", // MmAllocateNonCachedMemory(size)
+        "MmAllocateMappingAddress", // MmAllocateMappingAddress(size, tag)
+        // --- Windows COM allocation
+        "CoTaskMemAlloc", // CoTaskMemAlloc(size)
+        // --- Solaris/BSD kernel memory allocator
+        "kmem_alloc", // kmem_alloc(size, flags)
+        "kmem_zalloc", // kmem_zalloc(size, flags)
+        // --- OpenSSL memory allocation
+        "CRYPTO_malloc", // CRYPTO_malloc(size_t num, const char *file, int line)
+        "CRYPTO_zalloc", // CRYPTO_zalloc(size_t num, const char *file, int line)
+        "CRYPTO_secure_malloc", // CRYPTO_secure_malloc(size_t num, const char *file, int line)
+        "CRYPTO_secure_zalloc" // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
+      ]) and
+    sizeArg = 0
+    or
+    hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExAllocatePool", // ExAllocatePool(type, size)
+        "ExAllocatePoolWithTag", // ExAllocatePool(type, size, tag)
+        "ExAllocatePoolWithTagPriority", // ExAllocatePoolWithTagPriority(type, size, tag, priority)
+        "ExAllocatePoolWithQuota", // ExAllocatePoolWithQuota(type, size)
+        "ExAllocatePoolWithQuotaTag", // ExAllocatePoolWithQuotaTag(type, size, tag)
+        "IoAllocateMdl", // IoAllocateMdl(address, size, flag, flag, irp)
+        "IoAllocateErrorLogEntry", // IoAllocateErrorLogEntry(object, size)
+        // --- Windows Global / Local legacy allocation
+        "LocalAlloc", // LocalAlloc(flags, size)
+        "GlobalAlloc", // GlobalAlloc(flags, size)
+        // --- Windows System Services allocation
+        "VirtualAlloc" // VirtualAlloc(address, size, type, flag)
+      ]) and
+    sizeArg = 1
+    or
+    hasGlobalName(["HeapAlloc"]) and // HeapAlloc(heap, flags, size)
+    sizeArg = 2
+    or
+    hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "MmAllocatePagesForMdl", // MmAllocatePagesForMdl(minaddress, maxaddress, skip, size)
+        "MmAllocatePagesForMdlEx", // MmAllocatePagesForMdlEx(minaddress, maxaddress, skip, size, type, flags)
+        "MmAllocateNodePagesForMdlEx" // MmAllocateNodePagesForMdlEx(minaddress, maxaddress, skip, size, type, prefer, flags)
+      ]) and
+    sizeArg = 3
   }
 
   override int getSizeArg() { result = sizeArg }
@@ -112,20 +75,16 @@ class MallocAllocationFunction extends AllocationFunction {
  * An allocation function (such as `alloca`) that does not require a
  * corresponding free (and has an argument for the size in bytes).
  */
-class AllocaAllocationFunction extends AllocationFunction {
+private class AllocaAllocationFunction extends AllocationFunction {
   int sizeArg;
 
   AllocaAllocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // alloca(size)
-        name = "alloca" and sizeArg = 0
-        or
-        // __builtin_alloca(size)
-        name = "__builtin_alloca" and sizeArg = 0
-      )
-    )
+    hasGlobalName([
+        // --- stack allocation
+        "alloca", // // alloca(size)
+        "__builtin_alloca" // __builtin_alloca(size)
+      ]) and
+    sizeArg = 0
   }
 
   override int getSizeArg() { result = sizeArg }
@@ -137,16 +96,15 @@ class AllocaAllocationFunction extends AllocationFunction {
  * An allocation function (such as `calloc`) that has an argument for the size
  * and another argument for the size of those units (in bytes).
  */
-class CallocAllocationFunction extends AllocationFunction {
+private class CallocAllocationFunction extends AllocationFunction {
   int sizeArg;
   int multArg;
 
   CallocAllocationFunction() {
-    exists(string name |
-      hasGlobalOrStdName(name) and
-      // calloc(num, size)
-      (name = "calloc" and sizeArg = 1 and multArg = 0)
-    )
+    // --- C library allocation
+    hasGlobalOrStdName("calloc") and // calloc(num, size)
+    sizeArg = 1 and
+    multArg = 0
   }
 
   override int getSizeArg() { result = sizeArg }
@@ -158,34 +116,31 @@ class CallocAllocationFunction extends AllocationFunction {
  * An allocation function (such as `realloc`) that has an argument for the size
  * in bytes, and an argument for an existing pointer that is to be reallocated.
  */
-class ReallocAllocationFunction extends AllocationFunction {
+private class ReallocAllocationFunction extends AllocationFunction {
   int sizeArg;
   int reallocArg;
 
   ReallocAllocationFunction() {
-    exists(string name |
-      hasGlobalOrStdName(name) and
-      // realloc(ptr, size)
-      (name = "realloc" and sizeArg = 1 and reallocArg = 0)
-      or
-      hasGlobalName(name) and
-      (
-        // LocalReAlloc(ptr, size, flags)
-        name = "LocalReAlloc" and sizeArg = 1 and reallocArg = 0
-        or
-        // GlobalReAlloc(ptr, size, flags)
-        name = "GlobalReAlloc" and sizeArg = 1 and reallocArg = 0
-        or
-        // HeapReAlloc(heap, flags, ptr, size)
-        name = "HeapReAlloc" and sizeArg = 3 and reallocArg = 2
-        or
-        // CoTaskMemRealloc(ptr, size)
-        name = "CoTaskMemRealloc" and sizeArg = 1 and reallocArg = 0
-        or
-        // CRYPTO_realloc(void *addr, size_t num, const char *file, int line);
-        name = "CRYPTO_realloc" and sizeArg = 1 and reallocArg = 0
-      )
-    )
+    // --- C library allocation
+    hasGlobalOrStdName("realloc") and // realloc(ptr, size)
+    sizeArg = 1 and
+    reallocArg = 0
+    or
+    hasGlobalName([
+        // --- Windows Global / Local legacy allocation
+        "LocalReAlloc", // LocalReAlloc(ptr, size, flags)
+        "GlobalReAlloc", // GlobalReAlloc(ptr, size, flags)
+        // --- Windows COM allocation
+        "CoTaskMemRealloc", // CoTaskMemRealloc(ptr, size)
+        // --- OpenSSL memory allocation
+        "CRYPTO_realloc" // CRYPTO_realloc(void *addr, size_t num, const char *file, int line)
+      ]) and
+    sizeArg = 1 and
+    reallocArg = 0
+    or
+    hasGlobalName("HeapReAlloc") and // HeapReAlloc(heap, flags, ptr, size)
+    sizeArg = 3 and
+    reallocArg = 2
   }
 
   override int getSizeArg() { result = sizeArg }
@@ -197,76 +152,22 @@ class ReallocAllocationFunction extends AllocationFunction {
  * A miscellaneous allocation function that has no explicit argument for
  * the size of the allocation.
  */
-class SizelessAllocationFunction extends AllocationFunction {
+private class SizelessAllocationFunction extends AllocationFunction {
   SizelessAllocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // ExAllocateFromLookasideListEx(list)
-        name = "ExAllocateFromLookasideListEx"
-        or
-        // ExAllocateFromPagedLookasideList(list)
-        name = "ExAllocateFromPagedLookasideList"
-        or
-        // ExAllocateFromNPagedLookasideList(list)
-        name = "ExAllocateFromNPagedLookasideList"
-        or
-        // ExAllocateTimer(callback, context, attributes)
-        name = "ExAllocateTimer"
-        or
-        // IoAllocateWorkItem(object)
-        name = "IoAllocateWorkItem"
-        or
-        // MmMapLockedPagesWithReservedMapping(address, tag, list, type)
-        name = "MmMapLockedPagesWithReservedMapping"
-        or
-        // MmMapLockedPages(list, mode)
-        name = "MmMapLockedPages"
-        or
-        // MmMapLockedPagesSpecifyCache(list, mode, type, address, flag, flag)
-        name = "MmMapLockedPagesSpecifyCache"
-        or
-        // pool_get(pool, flags)
-        name = "pool_get"
-        or
-        // pool_cache_get(pool, flags)
-        name = "pool_cache_get"
-      )
-    )
-  }
-}
-
-/**
- * An `operator new` or `operator new[]` function that may be associated with `new` or
- * `new[]` expressions.  Note that `new` and `new[]` are not function calls, but these
- * functions may also be called directly.
- */
-class OperatorNewAllocationFunction extends AllocationFunction {
-  OperatorNewAllocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // operator new(bytes, ...)
-        name = "operator new"
-        or
-        // operator new[](bytes, ...)
-        name = "operator new[]"
-      )
-    )
-  }
-
-  override int getSizeArg() { result = 0 }
-
-  override predicate requiresDealloc() { not exists(getPlacementArgument()) }
-
-  /**
-   * Gets the position of the placement pointer if this is a placement
-   * `operator new` function.
-   */
-  int getPlacementArgument() {
-    getNumberOfParameters() = 2 and
-    getParameter(1).getType() instanceof VoidPointerType and
-    result = 1
+    hasGlobalName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExAllocateFromLookasideListEx", // ExAllocateFromLookasideListEx(list)
+        "ExAllocateFromPagedLookasideList", // ExAllocateFromPagedLookasideList(list)
+        "ExAllocateFromNPagedLookasideList", // ExAllocateFromNPagedLookasideList(list)
+        "ExAllocateTimer", // ExAllocateTimer(callback, context, attributes)
+        "IoAllocateWorkItem", // IoAllocateWorkItem(object)
+        "MmMapLockedPagesWithReservedMapping", // MmMapLockedPagesWithReservedMapping(address, tag, list, type)
+        "MmMapLockedPages", // MmMapLockedPages(list, mode)
+        "MmMapLockedPagesSpecifyCache", // MmMapLockedPagesSpecifyCache(list, mode, type, address, flag, flag)
+        // --- NetBSD pool manager
+        "pool_get", // pool_get(pool, flags)
+        "pool_cache_get" // pool_cache_get(pool, flags)
+      ])
   }
 }
 
@@ -302,7 +203,7 @@ private predicate deconstructSizeExpr(Expr sizeExpr, Expr lengthExpr, int sizeof
 /**
  * An allocation expression that is a function call, such as call to `malloc`.
  */
-class CallAllocationExpr extends AllocationExpr, FunctionCall {
+private class CallAllocationExpr extends AllocationExpr, FunctionCall {
   AllocationFunction target;
 
   CallAllocationExpr() {
@@ -353,7 +254,7 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall {
 /**
  * An allocation expression that is a `new` expression.
  */
-class NewAllocationExpr extends AllocationExpr, NewExpr {
+private class NewAllocationExpr extends AllocationExpr, NewExpr {
   NewAllocationExpr() { this instanceof NewExpr }
 
   override int getSizeBytes() { result = getAllocatedType().getSize() }
@@ -366,7 +267,7 @@ class NewAllocationExpr extends AllocationExpr, NewExpr {
 /**
  * An allocation expression that is a `new []` expression.
  */
-class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
+private class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
   NewArrayAllocationExpr() { this instanceof NewArrayExpr }
 
   override Expr getSizeExpr() {

cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll

@@ -9,112 +9,56 @@ import semmle.code.cpp.models.interfaces.Deallocation
 /**
  * A deallocation function such as `free`.
  */
-class StandardDeallocationFunction extends DeallocationFunction {
+private class StandardDeallocationFunction extends DeallocationFunction {
   int freedArg;
 
   StandardDeallocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        name = "free" and freedArg = 0
-        or
-        name = "realloc" and freedArg = 0
-        or
-        name = "CRYPTO_free" and freedArg = 0
-        or
-        name = "CRYPTO_secure_free" and freedArg = 0
-      )
-      or
-      hasGlobalOrStdName(name) and
-      (
-        name = "ExFreePoolWithTag" and freedArg = 0
-        or
-        name = "ExFreeToLookasideListEx" and freedArg = 1
-        or
-        name = "ExFreeToPagedLookasideList" and freedArg = 1
-        or
-        name = "ExFreeToNPagedLookasideList" and freedArg = 1
-        or
-        name = "ExDeleteTimer" and freedArg = 0
-        or
-        name = "IoFreeMdl" and freedArg = 0
-        or
-        name = "IoFreeWorkItem" and freedArg = 0
-        or
-        name = "IoFreeErrorLogEntry" and freedArg = 0
-        or
-        name = "MmFreeContiguousMemory" and freedArg = 0
-        or
-        name = "MmFreeContiguousMemorySpecifyCache" and freedArg = 0
-        or
-        name = "MmFreeNonCachedMemory" and freedArg = 0
-        or
-        name = "MmFreeMappingAddress" and freedArg = 0
-        or
-        name = "MmFreePagesFromMdl" and freedArg = 0
-        or
-        name = "MmUnmapReservedMapping" and freedArg = 0
-        or
-        name = "MmUnmapLockedPages" and freedArg = 0
-        or
-        name = "LocalFree" and freedArg = 0
-        or
-        name = "GlobalFree" and freedArg = 0
-        or
-        name = "HeapFree" and freedArg = 2
-        or
-        name = "VirtualFree" and freedArg = 0
-        or
-        name = "CoTaskMemFree" and freedArg = 0
-        or
-        name = "SysFreeString" and freedArg = 0
-        or
-        name = "LocalReAlloc" and freedArg = 0
-        or
-        name = "GlobalReAlloc" and freedArg = 0
-        or
-        name = "HeapReAlloc" and freedArg = 2
-        or
-        name = "CoTaskMemRealloc" and freedArg = 0
-        or
-        name = "kmem_free" and freedArg = 0
-        or
-        name = "pool_put" and freedArg = 1
-        or
-        name = "pool_cache_put" and freedArg = 1
-      )
-    )
+    hasGlobalName([
+        // --- C library allocation
+        "free", "realloc",
+        // --- OpenSSL memory allocation
+        "CRYPTO_free", "CRYPTO_secure_free"
+      ]) and
+    freedArg = 0
+    or
+    hasGlobalOrStdName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExFreePoolWithTag", "ExDeleteTimer", "IoFreeMdl", "IoFreeWorkItem", "IoFreeErrorLogEntry",
+        "MmFreeContiguousMemory", "MmFreeContiguousMemorySpecifyCache", "MmFreeNonCachedMemory",
+        "MmFreeMappingAddress", "MmFreePagesFromMdl", "MmUnmapReservedMapping",
+        "MmUnmapLockedPages",
+        // --- Windows Global / Local legacy allocation
+        "LocalFree", "GlobalFree", "LocalReAlloc", "GlobalReAlloc",
+        // --- Windows System Services allocation
+        "VirtualFree",
+        // --- Windows COM allocation
+        "CoTaskMemFree", "CoTaskMemRealloc",
+        // --- Windows Automation
+        "SysFreeString",
+        // --- Solaris/BSD kernel memory allocator
+        "kmem_free"
+      ]) and
+    freedArg = 0
+    or
+    hasGlobalOrStdName([
+        // --- Windows Memory Management for Windows Drivers
+        "ExFreeToLookasideListEx", "ExFreeToPagedLookasideList", "ExFreeToNPagedLookasideList",
+        // --- NetBSD pool manager
+        "pool_put", "pool_cache_put"
+      ]) and
+    freedArg = 1
+    or
+    hasGlobalOrStdName(["HeapFree", "HeapReAlloc"]) and
+    freedArg = 2
   }
 
   override int getFreedArg() { result = freedArg }
 }
 
-/**
- * An `operator delete` or `operator delete[]` function that may be associated
- * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`
- * are not function calls, but these functions may also be called directly.
- */
-class OperatorDeleteDeallocationFunction extends DeallocationFunction {
-  OperatorDeleteDeallocationFunction() {
-    exists(string name |
-      hasGlobalName(name) and
-      (
-        // operator delete(pointer, ...)
-        name = "operator delete"
-        or
-        // operator delete[](pointer, ...)
-        name = "operator delete[]"
-      )
-    )
-  }
-
-  override int getFreedArg() { result = 0 }
-}
-
 /**
  * An deallocation expression that is a function call, such as call to `free`.
  */
-class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
+private class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
   DeallocationFunction target;
 
   CallDeallocationExpr() { target = getTarget() }
@@ -125,7 +69,7 @@ class CallDeallocationExpr extends DeallocationExpr, FunctionCall {
 /**
  * An deallocation expression that is a `delete` expression.
  */
-class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
+private class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
   DeleteDeallocationExpr() { this instanceof DeleteExpr }
 
   override Expr getFreedExpr() { result = getExpr() }
@@ -134,7 +78,7 @@ class DeleteDeallocationExpr extends DeallocationExpr, DeleteExpr {
 /**
  * An deallocation expression that is a `delete []` expression.
  */
-class DeleteArrayDeallocationExpr extends DeallocationExpr, DeleteArrayExpr {
+private class DeleteArrayDeallocationExpr extends DeallocationExpr, DeleteArrayExpr {
   DeleteArrayDeallocationExpr() { this instanceof DeleteArrayExpr }
 
   override Expr getFreedExpr() { result = getExpr() }

cpp/ql/src/semmle/code/cpp/models/implementations/Fread.qll

@@ -1,7 +1,7 @@
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.FlowSource
 
-class Fread extends AliasFunction, RemoteFlowFunction {
+private class Fread extends AliasFunction, RemoteFlowFunction {
   Fread() { this.hasGlobalName("fread") }
 
   override predicate parameterNeverEscapes(int n) {

cpp/ql/src/semmle/code/cpp/models/implementations/GetDelim.qll

@@ -6,7 +6,8 @@ import semmle.code.cpp.models.interfaces.FlowSource
 /**
  * The standard functions `getdelim`, `getwdelim` and the glibc variant `__getdelim`.
  */
-class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction, RemoteFlowFunction {
+private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectFunction,
+  RemoteFlowFunction {
   GetDelimFunction() { hasGlobalName(["getdelim", "getwdelim", "__getdelim"]) }
 
   override predicate hasTaintFlow(FunctionInput i, FunctionOutput o) {

cpp/ql/src/semmle/code/cpp/models/implementations/Getenv.qll

@@ -0,0 +1,21 @@
+/**
+ * Provides an implementation class modeling the POSIX function `getenv`.
+ */
+
+import cpp
+import semmle.code.cpp.models.interfaces.FlowSource
+
+/**
+ * The POSIX function `getenv`.
+ */
+class Getenv extends LocalFlowFunction {
+  Getenv() { this.hasGlobalName("getenv") }
+
+  override predicate hasLocalFlowSource(FunctionOutput output, string description) {
+    (
+      output.isReturnValueDeref() or
+      output.isReturnValue()
+    ) and
+    description = "an environment variable"
+  }
+}

cpp/ql/src/semmle/code/cpp/models/implementations/Gets.qll

@@ -13,7 +13,7 @@ import semmle.code.cpp.models.interfaces.FlowSource
 /**
  * The standard functions `gets` and `fgets`.
  */
-class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
+private class GetsFunction extends DataFlowFunction, TaintFunction, ArrayFunction, AliasFunction,
   SideEffectFunction, RemoteFlowFunction {
   GetsFunction() {
     // gets(str)

cpp/ql/src/semmle/code/cpp/models/implementations/IdentityFunction.qll

@@ -6,7 +6,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
 /**
  * The standard function templates `std::move` and `std::forward`.
  */
-class IdentityFunction extends DataFlowFunction, SideEffectFunction, AliasFunction {
+private class IdentityFunction extends DataFlowFunction, SideEffectFunction, AliasFunction {
   IdentityFunction() {
     this.getNamespace().getParentNamespace() instanceof GlobalNamespace and
     this.getNamespace().getName() = "std" and