Merge pull request #4844 from johnlugton/servicestack

Add provisional support for ServiceStack framework to feature branch

.github/workflows/codeql-analysis.yml

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

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

@@ -1,19 +1,16 @@
 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:
+     - main
+     - 'rc/**'
      - 'lgtm.com'
   pull_request:
     paths:
       - '.github/workflows/generate-query-help-docs.yml'
       - 'docs/codeql/query-help/**'
-      
+
 jobs:
   build:
     runs-on: ubuntu-latest
@@ -21,7 +18,7 @@ jobs:
     - name: Clone github/codeql
       uses: actions/checkout@v2
       with:
-        path: codeql
+        path: codeql 
     - name: Clone github/codeql-go
       uses: actions/checkout@v2
       with:

.gitignore

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

CONTRIBUTING.md

@@ -38,8 +38,6 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html).
 
-    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/install-pre-commit-hook.md) for instructions on how to install the hook.
-
 4. **Compilation**
 
     - Compilation of the query and any associated libraries and tests must be resilient to future development of the [supported](docs/supported-queries.md) libraries. This means that the functionality cannot use internal libraries, cannot depend on the output of `getAQlClass`, and cannot make use of regexp matching on `toString`.

config/identical-files.json

@@ -358,14 +358,6 @@
     "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",
@@ -417,12 +409,5 @@
     "java/ql/src/Metrics/Files/CommentedOutCodeReferences.qhelp",
     "javascript/ql/src/Comments/CommentedOutCodeReferences.qhelp",
     "python/ql/src/Lexical/CommentedOutCodeReferences.qhelp"
-  ],
-  "IDE Contextual Queries": [
-    "cpp/ql/src/IDEContextual.qll",
-    "csharp/ql/src/IDEContextual.qll",
-    "java/ql/src/IDEContextual.qll",
-    "javascript/ql/src/IDEContextual.qll",
-    "python/ql/src/analysis/IDEContextual.qll"
   ]
 }

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

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

cpp/change-notes/2020-11-02-unused-local-variable.md

@@ -1,2 +0,0 @@
-lgtm,codescanning
-* Two issues causing the 'Unused local variable' query (`cpp/unused-local-variable`) to produce false positive results have been fixed.

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

@@ -1,4 +0,0 @@
-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

@@ -1,3 +0,0 @@
-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

@@ -1,2 +0,0 @@
-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

@@ -1,2 +0,0 @@
-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,8 +9,6 @@
 + semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql: /Correctness/Dangerous Conversions
 + semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
-+ semmlecode-cpp-queries/Likely Bugs/OO/UnsafeUseOfThis.ql: /Correctness/Dangerous Conversions
-+ semmlecode-cpp-queries/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql: /Correctness/Dangerous Conversions
   # Consistent Use
 + semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
 + semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

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

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

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

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

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

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

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

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

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

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

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

@@ -8,41 +8,168 @@ import semmle.code.cpp.AutogeneratedFile
 predicate trivialPositiveIntValue(string s) {
   // Small numbers
   s = [0 .. 20].toString() or
-  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"
-    ]
+  // 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"
 }
 
 predicate trivialIntValue(string s) {
@@ -108,7 +235,10 @@ predicate joiningStringTrivial(Literal lit) {
   // understand (which is against the spirit of these queries).
   stringLiteral(lit) and
   exists(FunctionCall fc |
-    fc.getTarget().getName() = ["operator+", "operator<<"] and
+    (
+      fc.getTarget().getName() = "operator+" or
+      fc.getTarget().getName() = "operator<<"
+    ) and
     fc.getAnArgument().getAChild*() = lit
   ) and
   lit.getValue().length() < 16
@@ -161,7 +291,8 @@ 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/Best Practices/Unused Entities/UnusedLocals.ql

@@ -57,12 +57,5 @@ where
   not declarationHasSideEffects(v) and
   not exists(AsmStmt s | f = s.getEnclosingFunction()) and
   not v.getAnAttribute().getName() = "unused" and
-  not any(ErrorExpr e).getEnclosingFunction() = f and // unextracted expr may use `v`
-  not exists(
-    Literal l // this case can be removed when the `myFunction2( [obj](){} );` test case doesn't depend on this exclusion
-  |
-    l.getEnclosingFunction() = f and
-    not exists(l.getValue())
-  ) and
-  not any(ConditionDeclExpr cde).getEnclosingFunction() = f // this case can be removed when the `if (a = b; a)` test case doesn't depend on this exclusion
+  not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr likely used `v`
 select v, "Variable " + v.getName() + " is not used"

cpp/ql/src/Critical/NewDelete.qll

@@ -5,6 +5,8 @@
 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,9 +59,14 @@ class Options extends string {
   predicate exits(Function f) {
     f.getAnAttribute().hasName("noreturn")
     or
-    f.hasGlobalOrStdName([
-        "exit", "_exit", "abort", "__assert_fail", "longjmp", "__builtin_unreachable"
-      ])
+    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"
+    )
     or
     CustomOptions::exits(f) // old Options.qll
   }

cpp/ql/src/IDEContextual.qll

@@ -1,22 +0,0 @@
-/**
- * Provides shared predicates related to contextual queries in the code viewer.
- */
-
-import semmle.files.FileSystem
-
-/**
- * Returns the `File` matching the given source file name as encoded by the VS
- * Code extension.
- */
-cached
-File getFileBySourceArchiveName(string name) {
-  // The name provided for a file in the source archive by the VS Code extension
-  // has some differences from the absolute path in the database:
-  // 1. colons are replaced by underscores
-  // 2. there's a leading slash, even for Windows paths: "C:/foo/bar" ->
-  //    "/C_/foo/bar"
-  // 3. double slashes in UNC prefixes are replaced with a single slash
-  // We can handle 2 and 3 together by unconditionally adding a leading slash
-  // before replacing double slashes.
-  name = ("/" + result.getAbsolutePath().replaceAll(":", "_")).replaceAll("//", "/")
-}

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

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

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

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

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

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

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

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

cpp/ql/src/JPL_C/Tasks.qll

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

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

@@ -13,17 +13,38 @@ import cpp
 import semmle.code.cpp.dataflow.DataFlow
 
 predicate whitelist(Function f) {
-  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_%")
+  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_%")
+    )
+  )
 }
 
 predicate whitelistPow(FunctionCall fc) {
-  fc.getTarget().getName() = ["pow", "powf", "powl"] and
+  (
+    fc.getTarget().getName() = "pow" or
+    fc.getTarget().getName() = "powf" or
+    fc.getTarget().getName() = "powl"
+  ) and
   exists(float value |
     value = fc.getArgument(0).getValue().toFloat() and
     (value.floor() - value).abs() < 0.001

cpp/ql/src/Likely Bugs/Leap Year/UncheckedReturnValueForTimeFunctions.ql

@@ -50,12 +50,10 @@ class SafeTimeGatheringFunction extends Function {
 class TimeConversionFunction extends Function {
   TimeConversionFunction() {
     this.getQualifiedName() =
-      [
-        "FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
-        "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
-        "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
-        "RtlTimeToSecondsSince1970", "_mkgmtime"
-      ]
+      ["FileTimeToSystemTime", "SystemTimeToFileTime", "SystemTimeToTzSpecificLocalTime",
+          "SystemTimeToTzSpecificLocalTimeEx", "TzSpecificLocalTimeToSystemTime",
+          "TzSpecificLocalTimeToSystemTimeEx", "RtlLocalTimeToSystemTime",
+          "RtlTimeToSecondsSince1970", "_mkgmtime"]
   }
 }
 

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

@@ -1,20 +0,0 @@
-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

@@ -1,30 +0,0 @@
-<!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

@@ -1,212 +0,0 @@
-/**
- * @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

@@ -1,48 +0,0 @@
-<!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

@@ -1,17 +0,0 @@
-/**
- * @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

@@ -1,13 +0,0 @@
-#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

@@ -1,13 +0,0 @@
-#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

@@ -1,50 +0,0 @@
-/**
- * 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

@@ -1,56 +0,0 @@
-/**
- * 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

@@ -1,48 +0,0 @@
-<!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

@@ -1,17 +0,0 @@
-/**
- * @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

@@ -1,59 +0,0 @@
-<!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

@@ -1,21 +0,0 @@
-/**
- * @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

@@ -1,24 +0,0 @@
-/**
- * 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

@@ -1,59 +0,0 @@
-<!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

@@ -1,20 +0,0 @@
-/**
- * @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

@@ -1,50 +0,0 @@
-/**
- * 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

@@ -1,51 +0,0 @@
-/**
- * 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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

@@ -189,7 +189,8 @@ 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
@@ -326,37 +327,52 @@ class InitializationFunction extends Function {
       // Return value is not a success code but the output functions never fail.
       name.matches("_Interlocked%")
       or
-      name =
-        [
-          // Functions that never fail, according to MSDN.
-          "QueryPerformanceCounter", "QueryPerformanceFrequency",
-          // Functions that never fail post-Vista, according to MSDN.
-          "InitializeCriticalSectionAndSpinCount",
-          // `rand_s` writes 0 to a non-null argument if it fails, according to MSDN.
-          "rand_s",
-          // IntersectRect initializes the argument regardless of whether the input intersects
-          "IntersectRect", "SetRect", "UnionRect",
-          // These functions appears to have an incorrect CFG, which leads to false positives
-          "PhysicalToLogicalDPIPoint", "LogicalToPhysicalDPIPoint",
-          // Sets NtProductType to default on error
-          "RtlGetNtProductType",
-          // Our CFG is not sophisticated enough to detect that the argument is always initialized
-          "StringCchLengthA",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlUnicodeToMultiByteSize",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlMultiByteToUnicodeSize",
-          // All paths init the argument, and always returns SUCCESS.
-          "RtlUnicodeToMultiByteN",
-          // Always initializes argument
-          "RtlGetFirstRange",
-          // Destination range is zeroed out on failure, assuming first two parameters are valid
-          "memcpy_s",
-          // This zeroes the memory unconditionally
-          "SeCreateAccessState",
-          // Argument initialization is optional, but always succeeds
-          "KeGetCurrentProcessorNumberEx"
-        ]
+      // 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"
     )
   }
 }
@@ -459,9 +475,12 @@ 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,9 +140,12 @@ class FopenCreationExpr extends FileCreationExpr {
 
 class FopensCreationExpr extends FileCreationExpr {
   FopensCreationExpr() {
-    this.getTarget().getName() = ["fopen_s", "_wfopen_s"] and
+    exists(string name | name = this.getTarget().getName() |
+      name = "fopen_s" or
+      name = "_wfopen_s"
+    ) and
     exists(string mode |
-      mode = ["w", "a"] and
+      (mode = "w" or mode = "a") and
       this.getArgument(2).getValue().matches(mode + "%")
     )
   }

cpp/ql/src/definitions.qll

@@ -4,7 +4,6 @@
  */
 
 import cpp
-import IDEContextual
 
 /**
  * Any element that might be the source or target of a jump-to-definition
@@ -208,3 +207,11 @@ Top definitionOf(Top e, string kind) {
   // later on.
   strictcount(result.getLocation()) < 10
 }
+
+/**
+ * Returns an appropriately encoded version of a filename `name`
+ * passed by the VS Code extension in order to coincide with the
+ * output of `.getFile()` on locatable entities.
+ */
+cached
+File getEncodedFile(string name) { result.getAbsolutePath().replaceAll(":", "_") = name }

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

@@ -13,10 +13,9 @@ 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 destination buffer, since the last character is the NULL terminator.
-    char buf2[20];
-    char buf3[10];
-    sscanf(buf2, "%9s", buf3);
+    // 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");
 
     // BAD, do not use scanf without specifying a length first
     char file[10];

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

cpp/ql/src/experimental/semmle/code/cpp/security/PrivateCleartextWrite.qll

@@ -7,6 +7,7 @@ import semmle.code.cpp.dataflow.TaintTracking
 import experimental.semmle.code.cpp.security.PrivateData
 import semmle.code.cpp.security.FileWrite
 import semmle.code.cpp.security.BufferWrite
+import semmle.code.cpp.dataflow.TaintTracking
 
 module PrivateCleartextWrite {
   /**

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 recommendation
+ * @problem.severity warning
  * @precision high
  * @id cpp/missing-header-guard
  * @tags efficiency

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

@@ -31,7 +31,8 @@ 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/localDefinitions.ql

@@ -12,5 +12,5 @@ import definitions
 external string selectedSourceFile();
 
 from Top e, Top def, string kind
-where def = definitionOf(e, kind) and e.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+where def = definitionOf(e, kind) and e.getFile() = getEncodedFile(selectedSourceFile())
 select e, def, kind

cpp/ql/src/localReferences.ql

@@ -12,6 +12,5 @@ import definitions
 external string selectedSourceFile();
 
 from Top e, Top def, string kind
-where
-  def = definitionOf(e, kind) and def.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+where def = definitionOf(e, kind) and def.getFile() = getEncodedFile(selectedSourceFile())
 select e, def, kind

cpp/ql/src/printAst.ql

@@ -22,6 +22,6 @@ class Cfg extends PrintASTConfiguration {
    * Print All functions from the selected file.
    */
   override predicate shouldPrintFunction(Function func) {
-    func.getFile() = getFileBySourceArchiveName(selectedSourceFile())
+    func.getFile() = getEncodedFile(selectedSourceFile())
   }
 }

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

@@ -35,7 +35,8 @@ 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,8 +236,9 @@ 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)))
     )
   }
 
@@ -976,12 +977,7 @@ class ClassTemplateInstantiation extends Class {
  * specialization - see `FullClassTemplateSpecialization` and
  * `PartialClassTemplateSpecialization`).
  */
-class ClassTemplateSpecialization extends Class {
-  ClassTemplateSpecialization() {
-    isFullClassTemplateSpecialization(this) or
-    isPartialClassTemplateSpecialization(this)
-  }
-
+abstract class ClassTemplateSpecialization extends Class {
   /**
    * Gets the primary template for the specialization, for example on
    * `S<T,int>`, the result is `S<T,U>`.
@@ -1001,16 +997,6 @@ 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`:
@@ -1027,31 +1013,19 @@ private predicate isFullClassTemplateSpecialization(Class c) {
  * ```
  */
 class FullClassTemplateSpecialization extends ClassTemplateSpecialization {
-  FullClassTemplateSpecialization() { isFullClassTemplateSpecialization(this) }
+  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
+  }
 
   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`:
@@ -1068,7 +1042,25 @@ private predicate isPartialClassTemplateSpecialization(Class c) {
  * ```
  */
 class PartialClassTemplateSpecialization extends ClassTemplateSpecialization {
-  PartialClassTemplateSpecialization() { isPartialClassTemplateSpecialization(this) }
+  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)))
+  }
 
   override string getAPrimaryQlClass() { result = "PartialClassTemplateSpecialization" }
 }

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

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

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

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

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

@@ -363,8 +363,20 @@ class File extends Container, @file {
  */
 class HeaderFile extends File {
   HeaderFile() {
-    this.getExtension().toLowerCase() =
-      ["h", "r", "hpp", "hxx", "h++", "hh", "hp", "tcc", "tpp", "txx", "t++"]
+    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++"
+      /*    ---         ---    */
+    )
     or
     not exists(this.getExtension()) and
     exists(Include i | i.getIncludedFile() = this)
@@ -394,7 +406,7 @@ class HeaderFile extends File {
  * `File.compiledAsC`.
  */
 class CFile extends File {
-  CFile() { this.getExtension().toLowerCase() = ["c", "i"] }
+  CFile() { exists(string ext | ext = this.getExtension().toLowerCase() | ext = "c" or ext = "i") }
 
   override string getAPrimaryQlClass() { result = "CFile" }
 }
@@ -407,10 +419,21 @@ class CFile extends File {
  */
 class CppFile extends File {
   CppFile() {
-    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.
+    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.
+    )
   }
 
   override string getAPrimaryQlClass() { result = "CppFile" }

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

@@ -334,18 +334,6 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
     )
   }
 
-  /**
-   * Gets the class of which this function, called `memberName`, is a member.
-   *
-   * Prefer to use `getDeclaringType()` or `getName()` directly if you do not
-   * need to reason about both.
-   */
-  pragma[nomagic]
-  Class getClassAndName(string memberName) {
-    this.hasName(memberName) and
-    this.getDeclaringType() = result
-  }
-
   /**
    * Implements `ControlFlowNode.getControlFlowScope`. The `Function` is
    * used to represent the exit node of the control flow graph, so it is
@@ -403,30 +391,20 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   /** Holds if this function has a `noexcept` exception specification. */
   predicate isNoExcept() { getADeclarationEntry().isNoExcept() }
 
-  /**
-   * Gets a function that overloads this one.
-   *
-   * Note: if _overrides_ are wanted rather than _overloads_ then
-   * `MemberFunction::getAnOverridingFunction` should be used instead.
-   */
+  /** Gets a function that overloads this one. */
   Function getAnOverload() {
-    (
-      // If this function is declared in a class, only consider other
-      // functions from the same class.
-      exists(string name, Class declaringType |
-        candGetAnOverloadMember(name, declaringType, this) and
-        candGetAnOverloadMember(name, declaringType, result)
-      )
-      or
-      // Conversely, if this function is not
-      // declared in a class, only consider other functions not declared in a
-      // class.
-      exists(string name, Namespace namespace |
-        candGetAnOverloadNonMember(name, namespace, this) and
-        candGetAnOverloadNonMember(name, namespace, result)
-      )
-    ) and
+    result.getName() = getName() and
+    result.getNamespace() = getNamespace() and
     result != this and
+    // If this function is declared in a class, only consider other
+    // functions from the same class. Conversely, if this function is not
+    // declared in a class, only consider other functions not declared in a
+    // class.
+    (
+      if exists(getDeclaringType())
+      then result.getDeclaringType() = getDeclaringType()
+      else not exists(result.getDeclaringType())
+    ) and
     // Instantiations and specializations don't participate in overload
     // resolution.
     not (
@@ -467,15 +445,50 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
       // ... and likewise for destructors.
       this.(Destructor).getADestruction().mayBeGloballyImpure()
     else
-      // 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"
-        ])
+      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"
+      )
   }
 
   /**
@@ -484,19 +497,6 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   override AccessHolder getEnclosingAccessHolder() { result = this.getDeclaringType() }
 }
 
-pragma[noinline]
-private predicate candGetAnOverloadMember(string name, Class declaringType, Function f) {
-  f.getName() = name and
-  f.getDeclaringType() = declaringType
-}
-
-pragma[noinline]
-private predicate candGetAnOverloadNonMember(string name, Namespace namespace, Function f) {
-  f.getName() = name and
-  f.getNamespace() = namespace and
-  not exists(f.getDeclaringType())
-}
-
 /**
  * A particular declaration or definition of a C/C++ function. For example the
  * declaration and definition of `MyFunction` in the following code are each a

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

@@ -205,21 +205,12 @@ class Constructor extends MemberFunction {
 /**
  * A function that defines an implicit conversion.
  */
-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")
-  }
-
+abstract class ImplicitConversionFunction extends MemberFunction {
   /** Gets the type this `ImplicitConversionFunction` takes as input. */
-  Type getSourceType() { none() } // overridden in subclasses
+  abstract Type getSourceType();
 
   /** Gets the type this `ImplicitConversionFunction` converts to. */
-  Type getDestType() { none() } // overridden in subclasses
+  abstract Type getDestType();
 }
 
 /**

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

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

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

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

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

@@ -60,7 +60,7 @@ private string getTemplateArgumentString(Declaration d, int i) {
 /**
  * A `Declaration` extended to add methods for generating strings useful only for dumps and debugging.
  */
-private class DumpDeclaration extends Declaration {
+abstract 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()
+      else result = basePrefix + " " + getSpecifierString().trim()
     )
   }
 
@@ -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()
+      then result = baseSuffix + " " + getSpecifierString().trim()
       else result = baseSuffix
     )
   }
@@ -385,7 +385,7 @@ private class DumpFunction extends DumpDeclaration, Function {
 
   private string getACVQualifier() {
     result = getASpecifier().getName() and
-    result = ["const", "volatile"]
+    (result = "const" or result = "volatile")
   }
 
   private string getDeclaratorSuffix() {

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

@@ -34,7 +34,8 @@ 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")
@@ -90,8 +91,7 @@ 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 and
-    shouldPrintFunction(stmt.getEnclosingFunction())
+    stmt.getADeclarationEntry() = entry
   } or
   TParametersNode(Function func) { shouldPrintFunction(func) } or
   TConstructorInitializersNode(Constructor ctor) {
@@ -234,27 +234,11 @@ 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(PrintableElement el) {
-  result = "[" + concat(el.getAPrimaryQlClass0(), ",") + "] "
+private string qlClass(ElementBase el) {
+  result = "[" + concat(el.getAPrimaryQlClass(), ",") + "] "
   // Alternative implementation -- do not delete. It is useful for QL class discovery.
   //result = "["+ concat(el.getAQlClass(), ",") + "] "
 }

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

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

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

@@ -274,7 +274,7 @@ class Type extends Locatable, @type {
 
   /**
    * Gets this type with any typedefs resolved. For example, given
-   * `typedef C T`, this would resolve `const T&` to `const C&`.
+   * `typedef C T`, this would resolve `const T&` to `const C&`.
    * Note that this will only work if the resolved type actually appears
    * on its own elsewhere in the program.
    */
@@ -577,9 +577,7 @@ class BoolType extends IntegralType {
  * unsigned char e, f;
  * ```
  */
-class CharType extends IntegralType {
-  CharType() { builtintypes(underlyingElement(this), _, [5, 6, 7], _, _, _) }
-}
+abstract class CharType extends IntegralType { }
 
 /**
  * The C/C++ `char` type (which is distinct from `signed char` and
@@ -1306,16 +1304,14 @@ 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() { result = concat(this.getASpecifier().getName(), " ") }
+  string getSpecifierString() { internalSpecString(this, result, 1) }
 
   override string explain() {
-    result = this.getSpecifierString() + " {" + this.getBaseType().explain() + "}"
+    result = this.getSpecifierString() + "{" + this.getBaseType().explain() + "}"
   }
 
   override predicate isDeeplyConst() {
@@ -1544,9 +1540,9 @@ class FunctionPointerIshType extends DerivedType {
 /**
  * A C++ pointer to data member. See 15.5.
  * ```
- * class C { public: int m; };
+ * class C { int m; };
  * int C::* p = &C::m;          // pointer to data member m of class C
- * class C c;
+ * class C *;
  * int val = c.*p;              // access data member
  * ```
  */
@@ -1714,6 +1710,28 @@ 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,11 +4,8 @@
 
 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. */
-class XMLLocatable extends @xmllocatable, TXMLLocatable {
+abstract class XMLLocatable extends @xmllocatable {
   /** Gets the source location for this element. */
   Location getLocation() { xmllocations(this, result) }
 
@@ -36,7 +33,7 @@ class XMLLocatable extends @xmllocatable, TXMLLocatable {
   }
 
   /** Gets a textual representation of this element. */
-  string toString() { none() } // overridden in subclasses
+  abstract string toString();
 }
 
 /**
@@ -54,7 +51,7 @@ class XMLParent extends @xmlparent {
    * Gets a printable representation of this XML parent.
    * (Intended to be overridden in subclasses.)
    */
-  string getName() { none() } // overridden in subclasses
+  abstract string getName();
 
   /** 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,7 +14,11 @@ class PackedTimeType extends Type {
   }
 }
 
-private predicate timeType(string typeName) { typeName = ["_SYSTEMTIME", "SYSTEMTIME", "tm"] }
+private predicate timeType(string typeName) {
+  typeName = "_SYSTEMTIME" or
+  typeName = "SYSTEMTIME" or
+  typeName = "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,6 +6,7 @@ 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__"] }
@@ -34,95 +35,66 @@ class AttributeFormattingFunction extends FormattingFunction {
 
 /**
  * A standard 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`.
+ * and a variable argument list of type `va_arg`.
  */
-predicate primitiveVariadicFormatter(
-  TopLevelFunction f, string type, int formatParamIndex, int outputParamIndex
-) {
-  type = f.getName().regexpCapture("_?_?va?([fs]?)n?w?printf(_s)?(_p)?(_l)?", 1) and
+predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
+  f.getName().regexpMatch("_?_?va?[fs]?n?w?printf(_s)?(_p)?(_l)?") and
   (
     if f.getName().matches("%\\_l")
     then formatParamIndex = f.getNumberOfParameters() - 3
     else formatParamIndex = f.getNumberOfParameters() - 2
-  ) and
-  (
-    if type = "" then outputParamIndex = -1 else outputParamIndex = 0 // Conveniently, these buffer parameters are all at index 0.
-  ) and
-  not (
-    // exclude functions with an implementation in the snapshot source
-    // directory, as they may not be standard implementations.
-    exists(f.getBlock()) and
-    exists(f.getFile().getRelativePath())
-  )
-}
-
-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
   )
 }
 
 /**
- * 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`.
+ * A standard function such as `vsprintf` that has an output parameter
+ * and a variable argument list of type `va_arg`.
  */
-predicate variadicFormatter(Function f, string type, int formatParamIndex, int outputParamIndex) {
-  primitiveVariadicFormatter(f, type, formatParamIndex, outputParamIndex)
+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)
   or
   not f.isVarargs() and
-  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, _)
+  callsVariadicFormatterOutput(f, outputParamIndex)
 }
 
 /**
  * Holds if `f` is a function such as `vprintf` that has a format parameter
  * (at `formatParamIndex`) and a variable argument list of type `va_arg`.
- *
- * DEPRECATED: Use the four argument version instead.
  */
-deprecated predicate variadicFormatter(Function f, int formatParamIndex) {
-  variadicFormatter(f, _, formatParamIndex, _)
+predicate variadicFormatter(Function f, int formatParamIndex) {
+  primitiveVariadicFormatter(f, formatParamIndex)
+  or
+  not f.isVarargs() and
+  callsVariadicFormatter(f, formatParamIndex)
 }
 
 /**
@@ -132,17 +104,11 @@ deprecated 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(boolean isStream) {
-    callsVariadicFormatter(this, "f", _, result) and isStream = true
-    or
-    callsVariadicFormatter(this, "s", _, result) and isStream = false
-  }
-
-  override predicate isOutputGlobal() { callsVariadicFormatter(this, "", _, _) }
+  override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
 }
 
 /**
@@ -908,7 +874,6 @@ class FormatLiteral extends Literal {
    */
   int getNumArgNeeded(int n) {
     exists(this.getConvSpecOffset(n)) and
-    exists(this.getConversionChar(n)) and
     result = count(int mode | hasFormatArgumentIndexFor(n, mode))
   }
 
@@ -1125,7 +1090,8 @@ 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))
   }
@@ -1141,7 +1107,8 @@ 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()