Ruby: Only use library inputs for libraries

Use the application heuristics to control whether we consider public
method parameters to all be sources of remote flow.

.gitattributes

@@ -52,6 +52,12 @@
 java/ql/test/stubs/**/*.java linguist-generated=true
 java/ql/test/experimental/stubs/**/*.java linguist-generated=true
 
+# For some languages, upgrade script testing references really old dbscheme
+# files from legacy upgrades that have CRLF line endings. Since upgrade
+# resolution relies on object hashes, we must suppress line ending conversion
+# for those testing dbscheme files.
+*/ql/lib/upgrades/initial/*.dbscheme -text
+
 # Generated test files - these are synced from the standard JavaScript libraries using
 # `javascript/ql/experimental/adaptivethreatmodeling/test/update_endpoint_test_files.py`.
 javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/autogenerated/**/*.js linguist-generated=true -merge

.github/workflows/check-change-note.yml

@@ -6,8 +6,11 @@ on:
     paths:
       - "*/ql/src/**/*.ql"
       - "*/ql/src/**/*.qll"
+      - "*/ql/lib/**/*.ql"
+      - "*/ql/lib/**/*.qll"
       - "!**/experimental/**"
       - "!ql/**"
+      - ".github/workflows/check-change-note.yml"
 
 jobs:
   check-change-note:

.github/workflows/js-ml-tests.yml

@@ -39,6 +39,12 @@ jobs:
 
       - uses: ./.github/actions/fetch-codeql
 
+      - name: Install pack dependencies
+        run: |
+          for pack in modelbuilding src; do
+            codeql pack install --mode verify -- "${pack}"
+          done
+
       - name: Check QL compilation
         run: |
           codeql query compile \
@@ -57,6 +63,9 @@ jobs:
 
       - uses: ./.github/actions/fetch-codeql
 
+      - name: Install pack dependencies
+        run: codeql pack install -- test
+
       - name: Run QL tests
         run: |
           codeql test run \

.github/workflows/ql-for-ql-build.yml

@@ -31,13 +31,13 @@ jobs:
         uses: actions/cache@v2
         with:
           path: ${{ runner.temp }}/query-pack.zip
-          key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
+          key: queries-${{ hashFiles('ql/**/*.ql*') }}-${{ hashFiles('ql/**/qlpack.yml') }}-${{ hashFiles('ql/ql/src/ql.dbscheme*') }}-${{ steps.get-codeql-version.outputs.version }}
       - name: Build query pack
         if: steps.cache-queries.outputs.cache-hit != 'true'
         run: |
           cd ql/ql/src
           "${CODEQL}" pack create
-          cd .codeql/pack/codeql/ql-all/0.0.0
+          cd .codeql/pack/codeql/ql/0.0.0
           zip "${PACKZIP}" -r .
         env:
           CODEQL: ${{ steps.find-codeql.outputs.codeql-path }}

.github/workflows/ruby-qltest.yml

@@ -63,6 +63,7 @@ jobs:
   qltest:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         slice: ["1/2", "2/2"]
     steps:

.github/workflows/validate-change-notes.yml

@@ -0,0 +1,29 @@
+name: Validate change notes
+
+on:
+  push:
+    paths:
+      - "*/ql/*/change-notes/**/*"
+      - ".github/workflows/validate-change-notes.yml"
+    branches:
+      - main
+      - "rc/*"
+  pull_request:
+    paths:
+      - "*/ql/*/change-notes/**/*"
+      - ".github/workflows/validate-change-notes.yml"
+
+jobs:
+  check-change-note:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+
+      - name: Setup CodeQL
+        uses: ./.github/actions/fetch-codeql
+
+      - name: Fail if there are any errors with existing change notes
+
+        run: |
+          codeql pack release --groups cpp,csharp,java,javascript,python,ruby,-examples,-test,-experimental

.pre-commit-config.yaml

@@ -0,0 +1,29 @@
+# See https://pre-commit.com for more information
+# See https://pre-commit.com/hooks.html for more hooks
+exclude: /test/.*$(?<!\.ql)(?<!\.qll)(?<!\.qlref)
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v3.2.0
+    hooks:
+    -   id: trailing-whitespace
+    -   id: end-of-file-fixer
+
+-   repo: local
+    hooks:
+    -   id: codeql-format
+        name: Fix QL file formatting
+        files: \.qll?$
+        language: system
+        entry: codeql query format --in-place
+
+    -   id: sync-files
+        name: Fix files required to be identical
+        language: system
+        entry: python3 config/sync-files.py --latest
+        pass_filenames: false
+
+    -   id: qhelp
+        name: Check query help generation
+        files: \.qhelp$
+        language: system
+        entry: python3 misc/scripts/check-qhelp.py

CONTRIBUTING.md

@@ -42,7 +42,11 @@ 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/pre-commit-hook-setup.md) for instructions on how to install the hook.
+    If you prefer, you can either:
+    1. install the [pre-commit framework](https://pre-commit.com/) and install the configured hooks on this repo via `pre-commit install`, or
+    2. use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted.
+
+    See the [pre-commit hook installation guide](docs/pre-commit-hook-setup.md) for instructions on the two approaches.
 
 4. **Compilation**
 

config/identical-files.json

@@ -426,7 +426,6 @@
     "python/ql/src/Lexical/CommentedOutCodeMetricOverview.inc.qhelp"
   ],
   "FLinesOfDuplicatedCodeCommon.inc.qhelp": [
-    "cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
     "java/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp",
     "javascript/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp",
     "python/ql/src/Metrics/FLinesOfDuplicatedCodeCommon.inc.qhelp"
@@ -465,7 +464,8 @@
   ],
   "SensitiveDataHeuristics Python/JS": [
     "javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
-    "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll"
+    "python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll",
+    "ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll"
   ],
   "ReDoS Util Python/JS/Ruby": [
     "javascript/ql/lib/semmle/javascript/security/performance/ReDoSUtil.qll",
@@ -501,5 +501,11 @@
     "javascript/ql/lib/tutorial.qll",
     "python/ql/lib/tutorial.qll",
     "ruby/ql/lib/tutorial.qll"
+  ],
+  "AccessPathSyntax": [
+    "csharp/ql/lib/semmle/code/csharp/dataflow/internal/AccessPathSyntax.qll",
+    "java/ql/lib/semmle/code/java/dataflow/internal/AccessPathSyntax.qll",
+    "javascript/ql/lib/semmle/javascript/frameworks/data/internal/AccessPathSyntax.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/AccessPathSyntax.qll"
   ]
 }

cpp/config/suites/c/correctness

@@ -31,6 +31,7 @@
 + semmlecode-cpp-queries/Critical/NewArrayDeleteMismatch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Critical/NewDeleteArrayMismatch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Critical/NewFreeMismatch.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql: /Correctness/Common Errors
   # Use of Libraries
 + semmlecode-cpp-queries/Likely Bugs/Memory Management/SuspiciousCallToMemset.ql: /Correctness/Use of Libraries
 + semmlecode-cpp-queries/Likely Bugs/Memory Management/SuspiciousSizeof.ql: /Correctness/Use of Libraries

cpp/config/suites/cpp/correctness

@@ -34,6 +34,7 @@
 + semmlecode-cpp-queries/Critical/NewArrayDeleteMismatch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Critical/NewDeleteArrayMismatch.ql: /Correctness/Common Errors
 + semmlecode-cpp-queries/Critical/NewFreeMismatch.ql: /Correctness/Common Errors
++ semmlecode-cpp-queries/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql: /Correctness/Common Errors
   # Exceptions
 + semmlecode-cpp-queries/Best Practices/Exceptions/AccidentalRethrow.ql: /Correctness/Exceptions
 + semmlecode-cpp-queries/Best Practices/Exceptions/CatchingByValue.ql: /Correctness/Exceptions

cpp/downgrades/2cd420191e5f782589b4e4efb70127de265390dd/upgrade.properties

@@ -0,0 +1,2 @@
+description: Remove unused legacy relations
+compatibility: backwards

cpp/downgrades/bb0f279f2acd793105a347d589b5afc8715d94c4/upgrade.properties

@@ -0,0 +1,3 @@
+description: Add relation for tracking variables from structured binding declarations
+compatibility: full
+is_structured_binding.rel: delete

cpp/downgrades/qlpack.yml

@@ -0,0 +1,4 @@
+name: codeql/cpp-downgrades
+groups: cpp
+downgrades: .
+library: true

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.0.10
+
+### New Features
+
+* Added a `isStructuredBinding` predicate to the `Variable` class which holds when the variable is declared as part of a structured binding declaration.
+
 ## 0.0.9
 
 ## 0.0.8

cpp/ql/lib/change-notes/2022-02-24-structured-bindings-in-ir.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Many queries now support structured bindings, as structured bindings are now handled in the IR translation.

cpp/ql/lib/change-notes/released/0.0.10.md

@@ -0,0 +1,5 @@
+## 0.0.10
+
+### New Features
+
+* Added a `isStructuredBinding` predicate to the `Variable` class which holds when the variable is declared as part of a structured binding declaration.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.9
+lastReleaseVersion: 0.0.10

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.0.9
+version: 0.0.11-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

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

@@ -169,6 +169,12 @@ class Variable extends Declaration, @variable {
     variable_instantiation(underlyingElement(this), unresolveElement(v))
   }
 
+  /**
+   * Holds if this variable is declated as part of a structured binding
+   * declaration. For example, `x` in `auto [x, y] = ...`.
+   */
+  predicate isStructuredBinding() { is_structured_binding(underlyingElement(this)) }
+
   /**
    * Holds if this is a compiler-generated variable. For example, a
    * [range-based for loop](http://en.cppreference.com/w/cpp/language/range-for)

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

@@ -353,9 +353,9 @@ module FlowVar_internal {
         // indirection.
         result = def.getAUse(v)
         or
-        exists(SsaDefinition descendentDef |
+        exists(SsaDefinition descendantDef |
-          this.getASuccessorSsaVar+() = TSsaVar(descendentDef, _) and
+          this.getASuccessorSsaVar+() = TSsaVar(descendantDef, _) and
-          result = descendentDef.getAUse(v)
+          result = descendantDef.getAUse(v)
         )
       )
       or

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

@@ -241,8 +241,8 @@ private module Cached {
     // For compatibility, send flow from arguments to parameters, even for
     // functions with no body.
     exists(FunctionCall call, int i |
-      sink.asExpr() = call.getArgument(i) and
+      sink.asExpr() = call.getArgument(pragma[only_bind_into](i)) and
-      result = resolveCall(call).getParameter(i)
+      result = resolveCall(call).getParameter(pragma[only_bind_into](i))
     )
     or
     // For compatibility, send flow into a `Variable` if there is flow to any

cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll

@@ -0,0 +1,270 @@
+/**
+ * This file provides a library for inter-procedural must-flow data flow analysis.
+ * Unlike `DataFlow.qll`, the analysis provided by this file checks whether data _must_ flow
+ * from a source to a _sink_.
+ */
+
+private import cpp
+import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.IR
+
+/**
+ * A configuration of a data flow analysis that performs must-flow analysis. This is different
+ * from `DataFlow.qll` which performs may-flow analysis (i.e., it finds paths where the source _may_
+ * flow to the sink).
+ *
+ * Like in `DataFlow.qll`, each use of the `MustFlow.qll` library must define its own unique extension
+ * of this abstract class. To create a configuration, extend this class with a subclass whose
+ * characteristic predicate is a unique singleton string and override `isSource`, `isSink` (and
+ * `isAdditionalFlowStep` if additional steps are required).
+ */
+abstract class MustFlowConfiguration extends string {
+  bindingset[this]
+  MustFlowConfiguration() { any() }
+
+  /**
+   * Holds if `source` is a relevant data flow source.
+   */
+  abstract predicate isSource(DataFlow::Node source);
+
+  /**
+   * Holds if `sink` is a relevant data flow sink.
+   */
+  abstract predicate isSink(DataFlow::Node sink);
+
+  /**
+   * Holds if the additional flow step from `node1` to `node2` must be taken
+   * into account in the analysis.
+   */
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+  /**
+   * Holds if data must flow from `source` to `sink` for this configuration.
+   *
+   * The corresponding paths are generated from the end-points and the graph
+   * included in the module `PathGraph`.
+   */
+  final predicate hasFlowPath(MustFlowPathNode source, MustFlowPathSink sink) {
+    this.isSource(source.getNode()) and
+    source.getASuccessor+() = sink
+  }
+}
+
+/** Holds if `node` flows from a source. */
+pragma[nomagic]
+private predicate flowsFromSource(DataFlow::Node node, MustFlowConfiguration config) {
+  config.isSource(node)
+  or
+  exists(DataFlow::Node mid |
+    step(mid, node, config) and
+    flowsFromSource(mid, pragma[only_bind_into](config))
+  )
+}
+
+/** Holds if `node` flows to a sink. */
+pragma[nomagic]
+private predicate flowsToSink(DataFlow::Node node, MustFlowConfiguration config) {
+  flowsFromSource(node, pragma[only_bind_into](config)) and
+  (
+    config.isSink(node)
+    or
+    exists(DataFlow::Node mid |
+      step(node, mid, config) and
+      flowsToSink(mid, pragma[only_bind_into](config))
+    )
+  )
+}
+
+cached
+private module Cached {
+  /** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
+  private 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 initialization instruction that receives the value of `instr` in `f`.
+   */
+  private predicate flowIntoParameter(
+    Function f, int n, CallInstruction call, Instruction instr, InitializeParameterInstruction init
+  ) {
+    not f.isVirtual() and
+    call.getPositionalArgument(n) = instr and
+    f = call.getStaticCallTarget() and
+    getEnclosingNonVirtualFunctionInitializeParameter(init, f) and
+    init.getParameter().getIndex() = pragma[only_bind_into](pragma[only_bind_out](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]
+  private predicate getPositionalArgumentInitParam(
+    CallInstruction call, Instruction instr, InitializeParameterInstruction init, Function f
+  ) {
+    exists(int n |
+      parameterOf(_, f, n) and
+      flowIntoParameter(f, pragma[only_bind_into](pragma[only_bind_out](n)), call, instr, init)
+    )
+  }
+
+  /**
+   * Holds if `instr` is the qualifier to a call to the non-virtual function `f`, and
+   * `init` is the corresponding initialization instruction that receives the value of
+   * `instr` in `f`.
+   */
+  pragma[noinline]
+  private 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 `f` is the enclosing non-virtual function of `init`. */
+  private predicate getEnclosingNonVirtualFunctionInitializeParameter(
+    InitializeParameterInstruction init, Function f
+  ) {
+    not f.isVirtual() and
+    init.getEnclosingFunction() = f
+  }
+
+  /** Holds if `f` is the enclosing non-virtual function of `init`. */
+  private 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.
+   */
+  private predicate flowThroughCallable(Instruction argument, Instruction parameter) {
+    // Flow from an argument to a parameter
+    exists(CallInstruction call, InitializeParameterInstruction init | init = parameter |
+      getPositionalArgumentInitParam(call, argument, init, call.getStaticCallTarget())
+      or
+      getThisArgumentInitParam(call, argument, init, call.getStaticCallTarget())
+    )
+    or
+    // Flow from argument indirection to parameter indirection
+    exists(
+      CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
+    |
+      init = parameter and
+      read.getPrimaryInstruction() = call and
+      getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
+    |
+      exists(int n |
+        read.getSideEffectOperand().getAnyDef() = argument and
+        read.getIndex() = pragma[only_bind_into](n) and
+        init.getParameter().getIndex() = pragma[only_bind_into](n)
+      )
+      or
+      call.getThisArgument() = argument and
+      init.getIRVariable() instanceof IRThisVariable
+    )
+  }
+
+  private predicate instructionToOperandStep(Instruction instr, Operand operand) {
+    operand.getDef() = instr
+  }
+
+  /**
+   * Holds if data flows from `operand` to `instr`.
+   *
+   * This predicate ignores flow through `PhiInstruction`s to create a 'must flow' relation.
+   */
+  private predicate operandToInstructionStep(Operand operand, Instruction instr) {
+    instr.(CopyInstruction).getSourceValueOperand() = operand
+    or
+    instr.(ConvertInstruction).getUnaryOperand() = operand
+    or
+    instr.(CheckedConvertOrNullInstruction).getUnaryOperand() = operand
+    or
+    instr.(InheritanceConversionInstruction).getUnaryOperand() = operand
+    or
+    instr.(ChiInstruction).getTotalOperand() = operand
+  }
+
+  cached
+  predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    instructionToOperandStep(nodeFrom.asInstruction(), nodeTo.asOperand())
+    or
+    flowThroughCallable(nodeFrom.asInstruction(), nodeTo.asInstruction())
+    or
+    operandToInstructionStep(nodeFrom.asOperand(), nodeTo.asInstruction())
+  }
+}
+
+/** Holds if `nodeFrom` flows to `nodeTo`. */
+private predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, MustFlowConfiguration config) {
+  exists(config) and
+  Cached::step(nodeFrom, nodeTo)
+  or
+  config.isAdditionalFlowStep(nodeFrom, nodeTo)
+}
+
+private newtype TLocalPathNode =
+  MkLocalPathNode(DataFlow::Node n, MustFlowConfiguration config) {
+    flowsToSink(n, config) and
+    (
+      config.isSource(n)
+      or
+      exists(MustFlowPathNode mid | step(mid.getNode(), n, config))
+    )
+  }
+
+/** A `Node` that is in a path from a source to a sink. */
+class MustFlowPathNode extends TLocalPathNode {
+  DataFlow::Node n;
+
+  MustFlowPathNode() { this = MkLocalPathNode(n, _) }
+
+  /** Gets the underlying node. */
+  DataFlow::Node getNode() { result = n }
+
+  /** Gets a textual representation of this node. */
+  string toString() { result = n.toString() }
+
+  /** Gets the location of this element. */
+  Location getLocation() { result = n.getLocation() }
+
+  /** Gets a successor node, if any. */
+  MustFlowPathNode getASuccessor() {
+    step(this.getNode(), result.getNode(), this.getConfiguration())
+  }
+
+  /** Gets the associated configuration. */
+  MustFlowConfiguration getConfiguration() { this = MkLocalPathNode(_, result) }
+}
+
+private class MustFlowPathSink extends MustFlowPathNode {
+  MustFlowPathSink() { this.getConfiguration().isSink(this.getNode()) }
+}
+
+/**
+ * Provides the query predicates needed to include a graph in a path-problem query.
+ */
+module PathGraph {
+  private predicate reach(MustFlowPathNode n) {
+    n instanceof MustFlowPathSink or reach(n.getASuccessor())
+  }
+
+  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
+  query predicate edges(MustFlowPathNode a, MustFlowPathNode b) {
+    a.getASuccessor() = b and reach(b)
+  }
+
+  /** Holds if `n` is a node in the graph of data flow path explanations. */
+  query predicate nodes(MustFlowPathNode n, string key, string val) {
+    reach(n) and key = "semmle.label" and val = n.toString()
+  }
+}

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

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
    * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
    */
   pragma[noinline]
-  final IRBlock postPominanceFrontier() {
+  final IRBlock postDominanceFrontier() {
     this.postDominates(result.getASuccessor()) and
     not this.strictlyPostDominates(result)
   }

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/internal/ValueNumberingInternal.qll

@@ -106,6 +106,12 @@ private predicate filteredNumberableInstruction(Instruction instr) {
   or
   instr instanceof FieldAddressInstruction and
   count(instr.(FieldAddressInstruction).getField()) != 1
+  or
+  instr instanceof InheritanceConversionInstruction and
+  (
+    count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
+    count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
+  )
 }
 
 private predicate variableAddressValueNumber(
@@ -115,8 +121,7 @@ private predicate variableAddressValueNumber(
   // The underlying AST element is used as value-numbering key instead of the
   // `IRVariable` to work around a problem where a variable or expression with
   // multiple types gives rise to multiple `IRVariable`s.
-  instr.getIRVariable().getAST() = ast and
+  unique( | | instr.getIRVariable().getAST()) = ast
-  strictcount(instr.getIRVariable().getAST()) = 1
 }
 
 private predicate initializeParameterValueNumber(
@@ -133,8 +138,7 @@ private predicate constantValueNumber(
   ConstantInstruction instr, IRFunction irFunc, IRType type, string value
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  strictcount(instr.getResultIRType()) = 1 and
+  unique( | | instr.getResultIRType()) = type and
-  instr.getResultIRType() = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
+  unique( | | instr.getField()) = field and
-  strictcount(instr.getField()) = 1 and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
+  tvalueNumber(instr.getUnary()) = operand and
-  instr.getDerivedClass() = derivedClass and
+  unique( | | instr.getBaseClass()) = baseClass and
-  tvalueNumber(instr.getUnary()) = operand
+  unique( | | instr.getDerivedClass()) = derivedClass
 }
 
 private predicate loadTotalOverlapValueNumber(

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

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
    * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
    */
   pragma[noinline]
-  final IRBlock postPominanceFrontier() {
+  final IRBlock postDominanceFrontier() {
     this.postDominates(result.getASuccessor()) and
     not this.strictlyPostDominates(result)
   }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingInternal.qll

@@ -106,6 +106,12 @@ private predicate filteredNumberableInstruction(Instruction instr) {
   or
   instr instanceof FieldAddressInstruction and
   count(instr.(FieldAddressInstruction).getField()) != 1
+  or
+  instr instanceof InheritanceConversionInstruction and
+  (
+    count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
+    count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
+  )
 }
 
 private predicate variableAddressValueNumber(
@@ -115,8 +121,7 @@ private predicate variableAddressValueNumber(
   // The underlying AST element is used as value-numbering key instead of the
   // `IRVariable` to work around a problem where a variable or expression with
   // multiple types gives rise to multiple `IRVariable`s.
-  instr.getIRVariable().getAST() = ast and
+  unique( | | instr.getIRVariable().getAST()) = ast
-  strictcount(instr.getIRVariable().getAST()) = 1
 }
 
 private predicate initializeParameterValueNumber(
@@ -133,8 +138,7 @@ private predicate constantValueNumber(
   ConstantInstruction instr, IRFunction irFunc, IRType type, string value
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  strictcount(instr.getResultIRType()) = 1 and
+  unique( | | instr.getResultIRType()) = type and
-  instr.getResultIRType() = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
+  unique( | | instr.getField()) = field and
-  strictcount(instr.getField()) = 1 and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
+  tvalueNumber(instr.getUnary()) = operand and
-  instr.getDerivedClass() = derivedClass and
+  unique( | | instr.getBaseClass()) = baseClass and
-  tvalueNumber(instr.getUnary()) = operand
+  unique( | | instr.getDerivedClass()) = derivedClass
 }
 
 private predicate loadTotalOverlapValueNumber(

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll

@@ -71,7 +71,8 @@ newtype TInstructionTag =
   AsmTag() or
   AsmInputTag(int elementIndex) { exists(AsmStmt asm | exists(asm.getChild(elementIndex))) } or
   ThisAddressTag() or
-  ThisLoadTag()
+  ThisLoadTag() or
+  StructuredBindingAccessTag()
 
 class InstructionTag extends TInstructionTag {
   final string toString() { result = "Tag" }
@@ -221,4 +222,6 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = ThisAddressTag() and result = "ThisAddress"
   or
   tag = ThisLoadTag() and result = "ThisLoad"
+  or
+  tag = StructuredBindingAccessTag() and result = "StructuredBindingAccess"
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll

@@ -3,6 +3,7 @@ private import semmle.code.cpp.ir.implementation.IRType
 private import semmle.code.cpp.ir.implementation.Opcode
 private import semmle.code.cpp.ir.implementation.internal.OperandTag
 private import semmle.code.cpp.ir.internal.CppType
+private import semmle.code.cpp.ir.internal.IRUtilities
 private import semmle.code.cpp.ir.internal.TempVariableTag
 private import InstructionTag
 private import TranslatedCondition
@@ -813,7 +814,9 @@ abstract class TranslatedVariableAccess extends TranslatedNonConstantExpr {
 }
 
 class TranslatedNonFieldVariableAccess extends TranslatedVariableAccess {
-  TranslatedNonFieldVariableAccess() { not expr instanceof FieldAccess }
+  TranslatedNonFieldVariableAccess() {
+    not expr instanceof FieldAccess and not isNonReferenceStructuredBinding(expr.getTarget())
+  }
 
   override Instruction getFirstInstruction() {
     if exists(this.getQualifier())
@@ -860,6 +863,71 @@ class TranslatedFieldAccess extends TranslatedVariableAccess {
   }
 }
 
+/**
+ * The IR translation of a variable access of a structured binding, where the type
+ * of the structured binding is not of a reference type, e.g., `x0` and `x1`
+ * in `auto [x0, x1] = xs` where `xs` is an array. Although the type of the
+ * structured binding is a non-reference type, the structured binding behaves
+ * like a reference. Hence, the translation requires a `VariableAddress` followed
+ * by a `Load` instead of only a `VariableAddress` as produced by
+ * `TranslatedVariableAccess`.
+ */
+class TranslatedStructuredBindingVariableAccess extends TranslatedNonConstantExpr {
+  override VariableAccess expr;
+
+  TranslatedStructuredBindingVariableAccess() { isNonReferenceStructuredBinding(expr.getTarget()) }
+
+  override Instruction getFirstInstruction() {
+    // Structured bindings cannot be qualified.
+    result = this.getInstruction(StructuredBindingAccessTag())
+  }
+
+  override TranslatedElement getChild(int id) {
+    // Structured bindings cannot be qualified.
+    none()
+  }
+
+  override Instruction getResult() { result = this.getInstruction(LoadTag()) }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    tag = StructuredBindingAccessTag() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(LoadTag())
+    or
+    tag = LoadTag() and
+    kind instanceof GotoEdge and
+    result = this.getParent().getChildSuccessor(this)
+  }
+
+  override Instruction getChildSuccessor(TranslatedElement child) { none() }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    tag = StructuredBindingAccessTag() and
+    opcode instanceof Opcode::VariableAddress and
+    resultType = getTypeForGLValue(this.getLValueReferenceType())
+    or
+    tag = LoadTag() and
+    opcode instanceof Opcode::Load and
+    resultType = getTypeForPRValue(this.getLValueReferenceType())
+  }
+
+  private LValueReferenceType getLValueReferenceType() {
+    // The extractor ensures `result` exists when `isNonReferenceStructuredBinding(expr.getTarget())` holds.
+    result.getBaseType() = expr.getUnspecifiedType()
+  }
+
+  override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
+    tag = LoadTag() and
+    operandTag instanceof AddressOperandTag and
+    result = this.getInstruction(StructuredBindingAccessTag())
+  }
+
+  override IRVariable getInstructionVariable(InstructionTag tag) {
+    tag = StructuredBindingAccessTag() and
+    result = getIRUserVariable(expr.getEnclosingFunction(), expr.getTarget())
+  }
+}
+
 class TranslatedFunctionAccess extends TranslatedNonConstantExpr {
   override FunctionAccess expr;
 

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

@@ -200,7 +200,7 @@ class IRBlock extends IRBlockBase {
    * post-dominate block `B`, but block `A` does post-dominate an immediate successor of block `B`.
    */
   pragma[noinline]
-  final IRBlock postPominanceFrontier() {
+  final IRBlock postDominanceFrontier() {
     this.postDominates(result.getASuccessor()) and
     not this.strictlyPostDominates(result)
   }

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/internal/ValueNumberingInternal.qll

@@ -106,6 +106,12 @@ private predicate filteredNumberableInstruction(Instruction instr) {
   or
   instr instanceof FieldAddressInstruction and
   count(instr.(FieldAddressInstruction).getField()) != 1
+  or
+  instr instanceof InheritanceConversionInstruction and
+  (
+    count(instr.(InheritanceConversionInstruction).getBaseClass()) != 1 or
+    count(instr.(InheritanceConversionInstruction).getDerivedClass()) != 1
+  )
 }
 
 private predicate variableAddressValueNumber(
@@ -115,8 +121,7 @@ private predicate variableAddressValueNumber(
   // The underlying AST element is used as value-numbering key instead of the
   // `IRVariable` to work around a problem where a variable or expression with
   // multiple types gives rise to multiple `IRVariable`s.
-  instr.getIRVariable().getAST() = ast and
+  unique( | | instr.getIRVariable().getAST()) = ast
-  strictcount(instr.getIRVariable().getAST()) = 1
 }
 
 private predicate initializeParameterValueNumber(
@@ -133,8 +138,7 @@ private predicate constantValueNumber(
   ConstantInstruction instr, IRFunction irFunc, IRType type, string value
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  strictcount(instr.getResultIRType()) = 1 and
+  unique( | | instr.getResultIRType()) = type and
-  instr.getResultIRType() = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
+  unique( | | instr.getField()) = field and
-  strictcount(instr.getField()) = 1 and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
+  tvalueNumber(instr.getUnary()) = operand and
-  instr.getDerivedClass() = derivedClass and
+  unique( | | instr.getBaseClass()) = baseClass and
-  tvalueNumber(instr.getUnary()) = operand
+  unique( | | instr.getDerivedClass()) = derivedClass
 }
 
 private predicate loadTotalOverlapValueNumber(

cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll

@@ -11,6 +11,15 @@ private Type getDecayedType(Type type) {
   result.(PointerType).getBaseType() = type.(ArrayType).getBaseType()
 }
 
+/**
+ * Holds if the sepcified variable is a structured binding with a non-reference
+ * type.
+ */
+predicate isNonReferenceStructuredBinding(Variable v) {
+  v.isStructuredBinding() and
+  not v.getUnspecifiedType() instanceof ReferenceType
+}
+
 /**
  * Get the actual type of the specified variable, as opposed to the declared type.
  * This returns the type of the variable after any pointer decay is applied, and
@@ -30,6 +39,11 @@ Type getVariableType(Variable v) {
         result = v.getInitializer().getExpr().getType()
         or
         not exists(v.getInitializer()) and result = v.getType()
+      else
+        if isNonReferenceStructuredBinding(v)
+        then
+          // The extractor ensures `r` exists when `isNonReferenceStructuredBinding(v)` holds.
+          exists(LValueReferenceType r | r.getBaseType() = v.getUnspecifiedType() | result = r)
         else result = v.getType()
   )
 }

cpp/ql/lib/semmlecode.cpp.dbscheme

@@ -135,52 +135,11 @@ externalData(
     string value : string ref
 );
 
-/**
- * The date of the snapshot.
- */
-snapshotDate(unique date snapshotDate : date ref);
-
 /**
  * The source location of the snapshot.
  */
 sourceLocationPrefix(string prefix : string ref);
 
-/**
- * Data used by the 'duplicate code' detection.
- */
-duplicateCode(
-    unique int id : @duplication,
-    string relativePath : string ref,
-    int equivClass : int ref
-);
-
-/**
- * Data used by the 'similar code' detection.
- */
-similarCode(
-    unique int id : @similarity,
-    string relativePath : string ref,
-    int equivClass : int ref
-);
-
-/**
- * Data used by the 'duplicate code' and 'similar code' detection.
- */
-@duplication_or_similarity = @duplication | @similarity
-
-/**
- * Data used by the 'duplicate code' and 'similar code' detection.
- */
-#keyset[id, offset]
-tokens(
-    int id : @duplication_or_similarity ref,
-    int offset : int ref,
-    int beginLine : int ref,
-    int beginColumn : int ref,
-    int endLine : int ref,
-    int endColumn : int ref
-);
-
 /**
  * Information about packages that provide code used during compilation.
  * The `id` is just a unique identifier.
@@ -487,6 +446,7 @@ var_decl_specifiers(
     int id: @var_decl ref,
     string name: string ref
 )
+is_structured_binding(unique int id: @variable ref);
 
 type_decls(
     unique int id: @type_decl,

cpp/ql/lib/upgrades/018f430097e80bcf4b2786c989dae94b7d82b819/upgrade.properties

@@ -0,0 +1,6 @@
+description: Remove unused legacy relations
+compatibility: full
+snapshotDate.rel: delete
+duplicateCode.rel: delete
+similarCode.rel: delete
+tokens.rel: delete

cpp/ql/lib/upgrades/2cd420191e5f782589b4e4efb70127de265390dd/upgrade.properties

@@ -0,0 +1,2 @@
+description: Add relation for tracking variables from structured binding declarations
+compatibility: backwards

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

@@ -18,7 +18,6 @@ where
   not lv1.isCompilerGenerated() and
   not lv2.isCompilerGenerated() and
   not lv1.getParentScope().(BlockStmt).isInMacroExpansion() and
-  not lv2.getParentScope().(BlockStmt).isInMacroExpansion() and
+  not lv2.getParentScope().(BlockStmt).isInMacroExpansion()
-  not lv1.getName() = "(unnamed local variable)"
 select lv1, "Variable " + lv1.getName() + " hides another variable of the same name (on $@).", lv2,
   "line " + lv2.getLocation().getStartLine().toString()

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.0.10
+
+### Deprecated Classes
+
+* The `CodeDuplication.Copy`, `CodeDuplication.DuplicateBlock`, and `CodeDuplication.SimilarBlock` classes have been deprecated.
+
 ## 0.0.9
 
 ### New Queries

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.ql

@@ -18,17 +18,19 @@ import cpp
 // recomputing the IR.
 private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import semmle.code.cpp.ir.IR
-import semmle.code.cpp.ir.dataflow.DataFlow::DataFlow
+import semmle.code.cpp.ir.dataflow.MustFlow
+import PathGraph
 
 /** Holds if `f` has a name that we intrepret as evidence of intentionally returning the value of the stack pointer. */
 predicate intentionallyReturnsStackPointer(Function f) {
   f.getName().toLowerCase().matches(["%stack%", "%sp%"])
 }
 
-/**
+class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
- * Holds if `source` is a node that represents the use of a stack variable
+  ReturnStackAllocatedMemoryConfig() { this = "ReturnStackAllocatedMemoryConfig" }
- */
+
-predicate isSource(Node source) {
+  override predicate isSource(DataFlow::Node source) {
+    // Holds if `source` is a node that represents the use of a stack variable
     exists(VariableAddressInstruction var, Function func |
       var = source.asInstruction() and
       func = var.getEnclosingFunction() and
@@ -39,37 +41,24 @@ predicate isSource(Node source) {
       not any(ErrorExpr e).getEnclosingFunction() = func and
       not intentionallyReturnsStackPointer(func)
     )
-}
+  }
 
-/**
+  override predicate isSink(DataFlow::Node sink) {
- * Holds if `sink` is a node that represents the `StoreInstruction` that is subsequently used in
+    // Holds if `sink` is a node that represents the `StoreInstruction` that is subsequently used in
- * a `ReturnValueInstruction`. We use the `StoreInstruction` instead of the instruction that defines the
+    // a `ReturnValueInstruction`.
- * `ReturnValueInstruction`'s source value oprand because the former has better location information.
+    // We use the `StoreInstruction` instead of the instruction that defines the
- */
+    // `ReturnValueInstruction`'s source value oprand because the former has better location information.
-predicate isSink(Node sink) {
     exists(StoreInstruction store |
       store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
         IRReturnVariable and
       sink.asOperand() = store.getSourceValueOperand()
     )
-}
+  }
 
-/** Holds if `node1` _must_ flow to `node2`. */
+  /**
-predicate step(Node node1, Node node2) {
+   * This configuration intentionally conflates addresses of fields and their object, and pointer offsets
-  instructionToOperandStep(node1.asInstruction(), node2.asOperand())
+   * with their base pointer as this allows us to detect cases where an object's address flows to a
-  or
+   * return statement via a field. For example:
-  operandToInstructionStep(node1.asOperand(), node2.asInstruction())
-}
-
-predicate instructionToOperandStep(Instruction instr, Operand operand) { operand.getDef() = instr }
-
-/**
- * Holds if `operand` flows to the result of `instr`.
- *
- * This predicate ignores flow through `PhiInstruction`s to create a 'must flow' relation. It also
- * intentionally conflates addresses of fields and their object, and pointer offsets with their
- * base pointer as this allows us to detect cases where an object's address flows to a return statement
- * via a field. For example:
    *
    * ```cpp
    * struct S { int x, y };
@@ -79,96 +68,21 @@ predicate instructionToOperandStep(Instruction instr, Operand operand) { operand
    * }
    * ```
    */
-predicate operandToInstructionStep(Operand operand, Instruction instr) {
+  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-  instr.(CopyInstruction).getSourceValueOperand() = operand
+    node2.asInstruction().(FieldAddressInstruction).getObjectAddressOperand() = node1.asOperand()
     or
-  instr.(ConvertInstruction).getUnaryOperand() = operand
+    node2.asInstruction().(PointerOffsetInstruction).getLeftOperand() = node1.asOperand()
-  or
-  instr.(CheckedConvertOrNullInstruction).getUnaryOperand() = operand
-  or
-  instr.(InheritanceConversionInstruction).getUnaryOperand() = operand
-  or
-  instr.(FieldAddressInstruction).getObjectAddressOperand() = operand
-  or
-  instr.(PointerOffsetInstruction).getLeftOperand() = operand
-}
-
-/** Holds if a source node flows to `n`. */
-predicate branchlessLocalFlow0(Node n) {
-  isSource(n)
-  or
-  exists(Node mid |
-    branchlessLocalFlow0(mid) and
-    step(mid, n)
-  )
-}
-
-/** Holds if `n` is reachable through some source node, and `n` also eventually reaches a sink. */
-predicate branchlessLocalFlow1(Node n) {
-  branchlessLocalFlow0(n) and
-  (
-    isSink(n)
-    or
-    exists(Node mid |
-      branchlessLocalFlow1(mid) and
-      step(n, mid)
-    )
-  )
-}
-
-newtype TLocalPathNode =
-  TLocalPathNodeMid(Node n) {
-    branchlessLocalFlow1(n) and
-    (
-      isSource(n) or
-      exists(LocalPathNodeMid mid | step(mid.getNode(), n))
-    )
   }
-
-abstract class LocalPathNode extends TLocalPathNode {
-  Node n;
-
-  /** Gets the underlying node. */
-  Node getNode() { result = n }
-
-  /** Gets a textual representation of this node. */
-  string toString() { result = n.toString() }
-
-  /** Gets the location of this element. */
-  Location getLocation() { result = n.getLocation() }
-
-  /** Gets a successor `LocalPathNode`, if any. */
-  LocalPathNode getASuccessor() { step(this.getNode(), result.getNode()) }
 }
 
-class LocalPathNodeMid extends LocalPathNode, TLocalPathNodeMid {
+from
-  LocalPathNodeMid() { this = TLocalPathNodeMid(n) }
+  MustFlowPathNode source, MustFlowPathNode sink, VariableAddressInstruction var,
-}
+  ReturnStackAllocatedMemoryConfig conf
-
-class LocalPathNodeSink extends LocalPathNodeMid {
-  LocalPathNodeSink() { isSink(this.getNode()) }
-}
-
-/**
- * Holds if `source` is a source node, `sink` is a sink node, and there's flow
- * from `source` to `sink` using `step` relation.
- */
-predicate hasFlow(LocalPathNode source, LocalPathNodeSink sink) {
-  isSource(source.getNode()) and
-  source.getASuccessor+() = sink
-}
-
-predicate reach(LocalPathNode n) { n instanceof LocalPathNodeSink or reach(n.getASuccessor()) }
-
-query predicate edges(LocalPathNode a, LocalPathNode b) { a.getASuccessor() = b and reach(b) }
-
-query predicate nodes(LocalPathNode n, string key, string val) {
-  reach(n) and key = "semmle.label" and val = n.toString()
-}
-
-from LocalPathNode source, LocalPathNodeSink sink, VariableAddressInstruction var
 where
-  hasFlow(source, sink) and
+  conf.hasFlowPath(source, sink) and
-  source.getNode().asInstruction() = var
+  source.getNode().asInstruction() = var and
+  // Only raise an alert if we're returning from the _same_ callable as the on that
+  // declared the stack variable.
+  var.getEnclosingFunction() = sink.getNode().getEnclosingCallable()
 select sink.getNode(), source, sink, "May return stack-allocated memory from $@.", var.getAST(),
   var.getAST().toString()

cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.cpp

@@ -0,0 +1,25 @@
+static const int* xptr;
+
+void localAddressEscapes() {
+  int x = 0;
+  xptr = &x;
+}
+
+void example1() {
+  localAddressEscapes();
+  const int* x = xptr; // BAD: This pointer points to expired stack allocated memory.
+}
+
+void localAddressDoesNotEscape() {
+  int x = 0;
+  xptr = &x;
+  // ...
+  // use `xptr`
+  // ...
+  xptr = nullptr;
+}
+
+void example2() {
+  localAddressDoesNotEscape();
+  const int* x = xptr; // GOOD: This pointer does not point to expired memory.
+}

cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.qhelp

@@ -0,0 +1,49 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+This rule finds uses of pointers that likely point to local variables in
+expired stack frames. A pointer to a local variable is only valid
+until the function returns, after which it becomes a dangling pointer.
+</p>
+
+</overview>
+<recommendation>
+
+<ol>
+
+<li>
+If it is necessary to take the address of a local variable, then make
+sure that the address is only stored in memory that does not outlive
+the local variable. For example, it is safe to store the address in
+another local variable. Similarly, it is also safe to pass the address
+of a local variable to another function provided that the other
+function only uses it locally and does not store it in non-local
+memory.
+</li>
+<li>
+If it is necessary to store an address which will outlive the
+current function scope, then it should be allocated on the heap. Care
+should be taken to make sure that the memory is deallocated when it is
+no longer needed, particularly when using low-level memory management
+routines such as <tt>malloc</tt>/<tt>free</tt> or
+<tt>new</tt>/<tt>delete</tt>. Modern C++ applications often use smart
+pointers, such as <tt>std::shared_ptr</tt>, to reduce the chance of
+a memory leak.
+</li>
+</ol>
+
+</recommendation>
+<example>
+
+<sample src="UsingExpiredStackAddress.cpp" />
+
+</example>
+<references>
+
+<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Dangling_pointer">Dangling pointer</a>.</li>
+
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Memory Management/UsingExpiredStackAddress.ql

@@ -0,0 +1,348 @@
+/**
+ * @name Use of expired stack-address
+ * @description Accessing the stack-allocated memory of a function
+ *              after it has returned can lead to memory corruption.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9.3
+ * @precision high
+ * @id cpp/using-expired-stack-address
+ * @tags reliability
+ *       security
+ *       external/cwe/cwe-825
+ */
+
+import cpp
+// We don't actually use the global value numbering library in this query, but without it we end up
+// recomputing the IR.
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.ir.IR
+
+predicate instructionHasVariable(VariableAddressInstruction vai, StackVariable var, Function f) {
+  var = vai.getASTVariable() and
+  f = vai.getEnclosingFunction() and
+  // Pointer-to-member types aren't properly handled in the dbscheme.
+  not vai.getResultType() instanceof PointerToMemberType and
+  // Rule out FPs caused by extraction errors.
+  not any(ErrorExpr e).getEnclosingFunction() = f
+}
+
+/**
+ * Holds if `source` is the base address of an address computation whose
+ * result is stored in `address`.
+ */
+predicate stackPointerFlowsToUse(Instruction address, VariableAddressInstruction source) {
+  address = source and
+  instructionHasVariable(source, _, _)
+  or
+  stackPointerFlowsToUse(address.(CopyInstruction).getSourceValue(), source)
+  or
+  stackPointerFlowsToUse(address.(ConvertInstruction).getUnary(), source)
+  or
+  stackPointerFlowsToUse(address.(CheckedConvertOrNullInstruction).getUnary(), source)
+  or
+  stackPointerFlowsToUse(address.(InheritanceConversionInstruction).getUnary(), source)
+  or
+  stackPointerFlowsToUse(address.(FieldAddressInstruction).getObjectAddress(), source)
+  or
+  stackPointerFlowsToUse(address.(PointerOffsetInstruction).getLeft(), source)
+}
+
+/**
+ * A HashCons-like table for comparing addresses that are
+ * computed relative to some global variable.
+ */
+newtype TGlobalAddress =
+  TGlobalVariable(GlobalOrNamespaceVariable v) {
+    // Pointer-to-member types aren't properly handled in the dbscheme.
+    not v.getUnspecifiedType() instanceof PointerToMemberType
+  } or
+  TLoad(TGlobalAddress address) {
+    address = globalAddress(any(LoadInstruction load).getSourceAddress())
+  } or
+  TConversion(string kind, TGlobalAddress address, Type fromType, Type toType) {
+    kind = "unchecked" and
+    exists(ConvertInstruction convert |
+      uncheckedConversionTypes(convert, fromType, toType) and
+      address = globalAddress(convert.getUnary())
+    )
+    or
+    kind = "checked" and
+    exists(CheckedConvertOrNullInstruction convert |
+      checkedConversionTypes(convert, fromType, toType) and
+      address = globalAddress(convert.getUnary())
+    )
+    or
+    kind = "inheritance" and
+    exists(InheritanceConversionInstruction convert |
+      inheritanceConversionTypes(convert, fromType, toType) and
+      address = globalAddress(convert.getUnary())
+    )
+  } or
+  TFieldAddress(TGlobalAddress address, Field f) {
+    exists(FieldAddressInstruction fai |
+      fai.getField() = f and
+      address = globalAddress(fai.getObjectAddress())
+    )
+  }
+
+pragma[noinline]
+predicate uncheckedConversionTypes(ConvertInstruction convert, Type fromType, Type toType) {
+  fromType = convert.getUnary().getResultType() and
+  toType = convert.getResultType()
+}
+
+pragma[noinline]
+predicate checkedConversionTypes(CheckedConvertOrNullInstruction convert, Type fromType, Type toType) {
+  fromType = convert.getUnary().getResultType() and
+  toType = convert.getResultType()
+}
+
+pragma[noinline]
+predicate inheritanceConversionTypes(
+  InheritanceConversionInstruction convert, Type fromType, Type toType
+) {
+  fromType = convert.getUnary().getResultType() and
+  toType = convert.getResultType()
+}
+
+/** Gets the HashCons value of an address computed by `instr`, if any. */
+TGlobalAddress globalAddress(Instruction instr) {
+  result = TGlobalVariable(instr.(VariableAddressInstruction).getASTVariable())
+  or
+  not instr instanceof LoadInstruction and
+  result = globalAddress(instr.(CopyInstruction).getSourceValue())
+  or
+  exists(LoadInstruction load | instr = load |
+    result = TLoad(globalAddress(load.getSourceAddress()))
+  )
+  or
+  exists(ConvertInstruction convert, Type fromType, Type toType | instr = convert |
+    uncheckedConversionTypes(convert, fromType, toType) and
+    result = TConversion("unchecked", globalAddress(convert.getUnary()), fromType, toType)
+  )
+  or
+  exists(CheckedConvertOrNullInstruction convert, Type fromType, Type toType | instr = convert |
+    checkedConversionTypes(convert, fromType, toType) and
+    result = TConversion("checked", globalAddress(convert.getUnary()), fromType, toType)
+  )
+  or
+  exists(InheritanceConversionInstruction convert, Type fromType, Type toType | instr = convert |
+    inheritanceConversionTypes(convert, fromType, toType) and
+    result = TConversion("inheritance", globalAddress(convert.getUnary()), fromType, toType)
+  )
+  or
+  exists(FieldAddressInstruction fai | instr = fai |
+    result = TFieldAddress(globalAddress(fai.getObjectAddress()), fai.getField())
+  )
+  or
+  result = globalAddress(instr.(PointerOffsetInstruction).getLeft())
+}
+
+/** Gets a `StoreInstruction` that may be executed after executing `store`. */
+pragma[inline]
+StoreInstruction getAStoreStrictlyAfter(StoreInstruction store) {
+  exists(IRBlock block, int index1, int index2 |
+    block.getInstruction(index1) = store and
+    block.getInstruction(index2) = result and
+    index2 > index1
+  )
+  or
+  exists(IRBlock block1, IRBlock block2 |
+    store.getBlock() = block1 and
+    result.getBlock() = block2 and
+    block1.getASuccessor+() = block2
+  )
+}
+
+/**
+ * Holds if `store` copies the address of `f`'s local variable `var`
+ * into the address `globalAddress`.
+ */
+predicate stackAddressEscapes(
+  StoreInstruction store, StackVariable var, TGlobalAddress globalAddress, Function f
+) {
+  globalAddress = globalAddress(store.getDestinationAddress()) and
+  exists(VariableAddressInstruction vai |
+    instructionHasVariable(pragma[only_bind_into](vai), var, f) and
+    stackPointerFlowsToUse(store.getSourceValue(), vai)
+  ) and
+  // Ensure there's no subsequent store that overrides the global address.
+  not globalAddress = globalAddress(getAStoreStrictlyAfter(store).getDestinationAddress())
+}
+
+predicate blockStoresToAddress(
+  IRBlock block, int index, StoreInstruction store, TGlobalAddress globalAddress
+) {
+  block.getInstruction(index) = store and
+  globalAddress = globalAddress(store.getDestinationAddress())
+}
+
+/**
+ * Holds if `globalAddress` evaluates to the address of `var` (which escaped through `store` before
+ * returning through `call`) when control reaches `block`.
+ */
+predicate globalAddressPointsToStack(
+  StoreInstruction store, StackVariable var, CallInstruction call, IRBlock block,
+  TGlobalAddress globalAddress, boolean isCallBlock, boolean isStoreBlock
+) {
+  (
+    if blockStoresToAddress(block, _, _, globalAddress)
+    then isStoreBlock = true
+    else isStoreBlock = false
+  ) and
+  (
+    isCallBlock = true and
+    exists(Function f |
+      stackAddressEscapes(store, var, globalAddress, f) and
+      call.getStaticCallTarget() = f and
+      call.getBlock() = block
+    )
+    or
+    isCallBlock = false and
+    step(store, var, call, globalAddress, _, block)
+  )
+}
+
+pragma[inline]
+int getInstructionIndex(Instruction instr, IRBlock block) { block.getInstruction(result) = instr }
+
+predicate step(
+  StoreInstruction store, StackVariable var, CallInstruction call, TGlobalAddress globalAddress,
+  IRBlock pred, IRBlock succ
+) {
+  exists(boolean isCallBlock, boolean isStoreBlock |
+    // Only recurse if there is no store to `globalAddress` in `mid`.
+    globalAddressPointsToStack(store, var, call, pred, globalAddress, isCallBlock, isStoreBlock)
+  |
+    // Post domination ensures that `block` is always executed after `mid`
+    // Domination ensures that `mid` is always executed before `block`
+    isStoreBlock = false and
+    succ.immediatelyPostDominates(pred) and
+    pred.immediatelyDominates(succ)
+    or
+    exists(CallInstruction anotherCall, int anotherCallIndex |
+      anotherCall = pred.getInstruction(anotherCallIndex) and
+      succ.getFirstInstruction() instanceof EnterFunctionInstruction and
+      succ.getEnclosingFunction() = anotherCall.getStaticCallTarget() and
+      (if isCallBlock = true then getInstructionIndex(call, _) < anotherCallIndex else any()) and
+      (
+        if isStoreBlock = true
+        then
+          forex(int storeIndex | blockStoresToAddress(pred, storeIndex, _, globalAddress) |
+            anotherCallIndex < storeIndex
+          )
+        else any()
+      )
+    )
+  )
+}
+
+newtype TPathElement =
+  TStore(StoreInstruction store) { globalAddressPointsToStack(store, _, _, _, _, _, _) } or
+  TCall(CallInstruction call, IRBlock block) {
+    globalAddressPointsToStack(_, _, call, block, _, _, _)
+  } or
+  TMid(IRBlock block) { step(_, _, _, _, _, block) } or
+  TSink(LoadInstruction load, IRBlock block) {
+    exists(TGlobalAddress address |
+      globalAddressPointsToStack(_, _, _, block, address, _, _) and
+      block.getAnInstruction() = load and
+      globalAddress(load.getSourceAddress()) = address
+    )
+  }
+
+class PathElement extends TPathElement {
+  StoreInstruction asStore() { this = TStore(result) }
+
+  CallInstruction asCall(IRBlock block) { this = TCall(result, block) }
+
+  predicate isCall(IRBlock block) { exists(this.asCall(block)) }
+
+  IRBlock asMid() { this = TMid(result) }
+
+  LoadInstruction asSink(IRBlock block) { this = TSink(result, block) }
+
+  predicate isSink(IRBlock block) { exists(this.asSink(block)) }
+
+  string toString() {
+    result = [asStore().toString(), asCall(_).toString(), asMid().toString(), asSink(_).toString()]
+  }
+
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.asStore()
+        .getLocation()
+        .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    this.asCall(_)
+        .getLocation()
+        .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    this.asMid().getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    or
+    this.asSink(_)
+        .getLocation()
+        .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
+
+predicate isSink(LoadInstruction load, IRBlock block, int index, TGlobalAddress globalAddress) {
+  block.getInstruction(index) = load and
+  globalAddress(load.getSourceAddress()) = globalAddress
+}
+
+query predicate edges(PathElement pred, PathElement succ) {
+  // Store -> caller
+  globalAddressPointsToStack(pred.asStore(), _, succ.asCall(_), _, _, _, _)
+  or
+  // Call -> basic block
+  pred.isCall(succ.asMid())
+  or
+  // Special case for when the caller goes directly to the load with no steps
+  // across basic blocks (i.e., caller -> sink)
+  exists(IRBlock block |
+    pred.isCall(block) and
+    succ.isSink(block)
+  )
+  or
+  // Basic block -> basic block
+  step(_, _, _, _, pred.asMid(), succ.asMid())
+  or
+  // Basic block -> load
+  succ.isSink(pred.asMid())
+}
+
+from
+  StoreInstruction store, StackVariable var, LoadInstruction load, CallInstruction call,
+  IRBlock block, boolean isCallBlock, TGlobalAddress address, boolean isStoreBlock,
+  PathElement source, PathElement sink, int loadIndex
+where
+  globalAddressPointsToStack(store, var, call, block, address, isCallBlock, isStoreBlock) and
+  isSink(load, block, loadIndex, address) and
+  (
+    // We know that we have a sequence:
+    // (1) store to `address` -> (2) return from `f` -> (3) load from `address`.
+    // But if (2) and (3) happen in the sam block we need to check the
+    // block indices to ensure that (3) happens after (2).
+    if isCallBlock = true
+    then
+      // If so, the load must happen after the call.
+      getInstructionIndex(call, _) < loadIndex
+    else any()
+  ) and
+  (
+    // If there is a store to the address we need to make sure that the load we found was
+    // before that store (So that the load doesn't read an overwritten value).
+    if isStoreBlock = true
+    then
+      forex(int storeIndex | blockStoresToAddress(block, storeIndex, _, address) |
+        loadIndex < storeIndex
+      )
+    else any()
+  ) and
+  source.asStore() = store and
+  sink.asSink(_) = load
+select sink, source, sink, "Stack variable $@ escapes $@ and is used after it has expired.", var,
+  var.toString(), store, "here"

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

@@ -17,73 +17,32 @@
 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
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
-private import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.MustFlow
+import PathGraph
 
-bindingset[n, result]
+class UnsafeUseOfThisConfig extends MustFlowConfiguration {
-int unbind(int n) { result >= n and result <= n }
+  UnsafeUseOfThisConfig() { this = "UnsafeUseOfThisConfig" }
 
-/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
+  override predicate isSource(DataFlow::Node source) { isSource(source, _, _) }
-predicate parameterOf(Parameter p, Function f, int n) {
-  not f.isVirtual() and f.getParameter(n) = p
-}
 
-/**
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
- * 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) {
+predicate isSink(DataFlow::Node sink, CallInstruction call) {
   exists(PureVirtualFunction func |
     call.getStaticCallTarget() = func and
-    call.getThisArgument() = instr and
+    call.getThisArgument() = sink.asInstruction() 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) {
+predicate isSource(DataFlow::Node source, string msg, Class c) {
+  exists(InitializeParameterInstruction init | init = source.asInstruction() |
     (
       exists(Constructor func |
         not func instanceof CopyConstructor and
@@ -96,88 +55,7 @@ predicate isSource(InitializeParameterInstruction init, string msg, Class c) {
     ) 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)
 }
 
 /**
@@ -188,22 +66,19 @@ predicate successor(Instruction instr, Instruction succ) {
  * - `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
+  MustFlowPathNode source, string msg, Class sourceClass, MustFlowPathNode sink,
+  CallInstruction call
 ) {
-  isSource(source, msg, sourceClass) and
+  exists(UnsafeUseOfThisConfig conf |
-  flowsToSink(source, sink) and
+    conf.hasFlowPath(source, sink) and
-  isSink(sink, call)
+    isSource(source.getNode(), msg, sourceClass) and
+    isSink(sink.getNode(), call)
+  )
 }
 
-query predicate edges(Instruction a, Instruction b) { successor(a, b) and flowsToSink(b, _) }
+from
-
+  MustFlowPathNode source, MustFlowPathNode sink, CallInstruction call, string msg,
-query predicate nodes(Instruction n, string key, string val) {
+  Class sourceClass
-  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.

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCode.qhelp

@@ -1,6 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<include src="FLinesOfDuplicatedCodeCommon.inc.qhelp" />
-</qhelp>

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCode.ql

@@ -1,27 +0,0 @@
-/**
- * @deprecated
- * @name Duplicated lines in files
- * @description The number of lines in a file, including code, comment
- *              and whitespace lines, which are duplicated in at least
- *              one other place.
- * @kind treemap
- * @treemap.warnOn highValues
- * @metricType file
- * @metricAggregate avg sum max
- * @id cpp/duplicated-lines-in-files
- * @tags testability
- *       modularity
- */
-
-import external.CodeDuplication
-
-from File f, int n
-where
-  n =
-    count(int line |
-      exists(DuplicateBlock d | d.sourceFile() = f |
-        line in [d.sourceStartLine() .. d.sourceEndLine()]
-      ) and
-      not whitelistedLineForDuplication(f, line)
-    )
-select f, n order by n desc

cpp/ql/src/Metrics/Files/FLinesOfDuplicatedCodeCommon.inc.qhelp

@@ -1,35 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-
-<p>
-This metric measures the number of lines in a file that are contained within a block that is duplicated elsewhere. These lines may include code, comments and whitespace, and the duplicate block may be in this file or in another file.
-</p>
-
-<p>
-A file that contains many lines that are duplicated within the code base is problematic
-for a number of reasons.
-</p>
-
-</overview>
-<include src="DuplicationProblems.inc.qhelp" />
-
-<recommendation>
-
-<p>
-Refactor files with lots of duplicated code to extract the common code into
-a shared library or module.
-</p>
-
-</recommendation>
-<references> 
-
-
-<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Duplicate_code">Duplicate code</a>.</li>
-<li>M. Fowler, <em>Refactoring</em>. Addison-Wesley, 1999.</li>
-
-
-</references>
-</qhelp>

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -3,7 +3,7 @@
  * @description Non-HTTPS connections can be intercepted by third parties.
  * @kind path-problem
  * @problem.severity warning
- * @precision medium
+ * @precision high
  * @id cpp/non-https-url
  * @tags security
  *       external/cwe/cwe-319
@@ -12,6 +12,7 @@
 
 import cpp
 import semmle.code.cpp.dataflow.TaintTracking
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 import DataFlow::PathGraph
 
 /**
@@ -57,7 +58,12 @@ class HttpStringToUrlOpenConfig extends TaintTracking::Configuration {
 
   override predicate isSource(DataFlow::Node src) {
     // Sources are strings containing an HTTP URL not in a private domain.
-    src.asExpr() instanceof HttpStringLiteral
+    src.asExpr() instanceof HttpStringLiteral and
+    // block taint starting at `strstr`, which is likely testing an existing URL, rather than constructing an HTTP URL.
+    not exists(FunctionCall fc |
+      fc.getTarget().getName() = ["strstr", "strcasestr"] and
+      fc.getArgument(1) = globalValueNumber(src.asExpr()).getAnExpr()
+    )
   }
 
   override predicate isSink(DataFlow::Node sink) {

cpp/ql/src/Security/CWE/CWE-326/InsufficientKeySize.c

@@ -0,0 +1,8 @@
+void encrypt_with_openssl(EVP_PKEY_CTX *ctx) {
+
+  // BAD: only 1024 bits for an RSA key
+  EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 1024);
+
+  // GOOD: 2048 bits for an RSA key
+  EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 2048);
+}

cpp/ql/src/Security/CWE/CWE-326/InsufficientKeySize.qhelp

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Using cryptographic algorithms with a small key size can leave data vulnerable to being decrypted.</p>
+
+<p>Many cryptographic algorithms provided by cryptography libraries can be configured with key sizes that are
+vulnerable to brute force attacks. Using such a key size means that an attacker may be able to easily decrypt the
+encrypted data.</p>
+
+</overview>
+<recommendation>
+
+<p>Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048.</p>
+
+</recommendation>
+<example>
+
+<p>The following code shows an example of using the <code>openssl</code> library to generate an RSA key.
+When creating a key, you must specify which key size to use. The first example uses 1024 bits, which is not
+considered sufficient. The second example uses 2048 bits, which is currently considered sufficient.</p>
+
+<sample src="InsufficientKeySize.c" />
+
+</example>
+<references>
+
+<li>NIST, FIPS 140 Annex a: <a href="http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf">
+Approved Security Functions</a>.</li>
+<li>NIST, SP 800-131A: <a href="https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf">
+Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.</li>
+
+<!--  LocalWords:  CWE
+ -->
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql

@@ -0,0 +1,63 @@
+/**
+ * @name Use of a cryptographic algorithm with insufficient key size
+ * @description Using cryptographic algorithms with too small a key size can
+ *              allow an attacker to compromise security.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id cpp/insufficient-key-size
+ * @tags security
+ *       external/cwe/cwe-326
+ */
+
+import cpp
+import semmle.code.cpp.ir.dataflow.DataFlow
+import semmle.code.cpp.ir.IR
+import DataFlow::PathGraph
+
+// Gets the recommended minimum key size (in bits) of `func`, the name of an encryption function that accepts a key size as parameter `paramIndex`
+int getMinimumKeyStrength(string func, int paramIndex) {
+  func =
+    [
+      "EVP_PKEY_CTX_set_dsa_paramgen_bits", "DSA_generate_parameters_ex",
+      "EVP_PKEY_CTX_set_rsa_keygen_bits", "RSA_generate_key_ex", "RSA_generate_key_fips",
+      "EVP_PKEY_CTX_set_dh_paramgen_prime_len", "DH_generate_parameters_ex"
+    ] and
+  paramIndex = 1 and
+  result = 2048
+}
+
+class KeyStrengthFlow extends DataFlow::Configuration {
+  KeyStrengthFlow() { this = "KeyStrengthFlow" }
+
+  override predicate isSource(DataFlow::Node node) {
+    exists(int bits |
+      node.asInstruction().(IntegerConstantInstruction).getValue().toInt() = bits and
+      bits < getMinimumKeyStrength(_, _) and
+      bits > 0 // exclude sentinel values
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(FunctionCall fc, string name, int param |
+      node.asExpr() = fc.getArgument(param) and
+      fc.getTarget().hasGlobalName(name) and
+      exists(getMinimumKeyStrength(name, param))
+    )
+  }
+}
+
+from
+  DataFlow::PathNode source, DataFlow::PathNode sink, KeyStrengthFlow conf, FunctionCall fc,
+  int param, string name, int minimumBits, int bits
+where
+  conf.hasFlowPath(source, sink) and
+  sink.getNode().asExpr() = fc.getArgument(param) and
+  fc.getTarget().hasGlobalName(name) and
+  minimumBits = getMinimumKeyStrength(name, param) and
+  bits = source.getNode().asInstruction().(ConstantValueInstruction).getValue().toInt() and
+  bits < minimumBits and
+  bits != 0
+select fc, source, sink,
+  "The key size $@ is less than the recommended key size of " + minimumBits.toString() + " bits.",
+  source, bits.toString()

cpp/ql/src/Security/CWE/CWE-497/ExposedSystemData.ql

@@ -3,7 +3,7 @@
  * @description Exposing system data or debugging information helps
  *              an adversary learn about the system and form an
  *              attack plan.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @security-severity 6.5
  * @precision medium
@@ -14,7 +14,9 @@
 
 import cpp
 import semmle.code.cpp.commons.Environment
-import semmle.code.cpp.security.OutputWrite
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import semmle.code.cpp.models.interfaces.FlowSource
+import DataFlow::PathGraph
 
 /**
  * An element that should not be exposed to an adversary.
@@ -24,42 +26,19 @@ abstract class SystemData extends Element {
    * Gets an expression that is part of this `SystemData`.
    */
   abstract Expr getAnExpr();
-
-  /**
-   * Gets an expression whose value originates from, or is used by,
-   * this `SystemData`.
-   */
-  Expr getAnExprIndirect() {
-    // direct SystemData
-    result = this.getAnExpr() or
-    // flow via global or member variable (conservative approximation)
-    result = this.getAnAffectedVar().getAnAccess() or
-    // flow via stack variable
-    definitionUsePair(_, this.getAnExprIndirect(), result) or
-    useUsePair(_, this.getAnExprIndirect(), result) or
-    useUsePair(_, result, this.getAnExprIndirect()) or
-    // flow from assigned value to assignment expression
-    result.(AssignExpr).getRValue() = this.getAnExprIndirect()
-  }
-
-  /**
-   * Gets a global or member variable that may be affected by this system
-   * data (conservative approximation).
-   */
-  private Variable getAnAffectedVar() {
-    (
-      result.getAnAssignedValue() = this.getAnExprIndirect() or
-      result.getAnAccess() = this.getAnExprIndirect()
-    ) and
-    not result instanceof LocalScopeVariable
-  }
 }
 
 /**
  * Data originating from the environment.
  */
 class EnvData extends SystemData {
-  EnvData() { this instanceof EnvironmentRead }
+  EnvData() {
+    // identify risky looking environment variables only
+    this.(EnvironmentRead)
+        .getEnvironmentVariable()
+        .toLowerCase()
+        .regexpMatch(".*(user|host|admin|root|home|path|http|ssl|snmp|sock|port|proxy|pass|token|crypt|key).*")
+  }
 
   override Expr getAnExpr() { result = this }
 }
@@ -91,11 +70,6 @@ class SQLConnectInfo extends SystemData {
 }
 
 private predicate posixSystemInfo(FunctionCall source, Element use) {
-  // long sysconf(int name)
-  //  - various OS / system values and limits
-  source.getTarget().hasName("sysconf") and
-  use = source
-  or
   // size_t confstr(int name, char *buf, size_t len)
   //  - various OS / system strings, such as the libc version
   // int statvfs(const char *__path, struct statvfs *__buf)
@@ -311,70 +285,31 @@ class RegQuery extends SystemData {
   override Expr getAnExpr() { regQuery(this, result) }
 }
 
-/**
+class ExposedSystemDataConfiguration extends TaintTracking::Configuration {
- * Somewhere data is output.
+  ExposedSystemDataConfiguration() { this = "ExposedSystemDataConfiguration" }
- */
-abstract class DataOutput extends Element {
-  /**
-   * Get an expression containing data that is output.
-   */
-  abstract Expr getASource();
-}
 
-/**
+  override predicate isSource(DataFlow::Node source) {
- * Data that is output via standard output or standard error.
+    source.asConvertedExpr() = any(SystemData sd).getAnExpr()
- */
+  }
-class StandardOutput extends DataOutput instanceof OutputWrite {
-  override Expr getASource() { result = OutputWrite.super.getASource() }
-}
 
-private predicate socketCallOrIndirect(FunctionCall call) {
+  override predicate isSink(DataFlow::Node sink) {
-  // direct socket call
+    exists(FunctionCall fc, FunctionInput input, int arg |
-  // int socket(int domain, int type, int protocol);
+      fc.getTarget().(RemoteFlowSinkFunction).hasRemoteFlowSink(input, _) and
-  call.getTarget().getName() = "socket"
+      input.isParameterDeref(arg) and
-  or
+      fc.getArgument(arg).getAChild*() = sink.asExpr()
-  exists(ReturnStmt rtn |
-    // indirect socket call
-    call.getTarget() = rtn.getEnclosingFunction() and
-    (
-      socketCallOrIndirect(rtn.getExpr()) or
-      socketCallOrIndirect(rtn.getExpr().(VariableAccess).getTarget().getAnAssignedValue())
-    )
     )
+  }
 }
 
-private predicate socketFileDescriptor(Expr e) {
+from ExposedSystemDataConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
-  exists(Variable var, FunctionCall socket |
-    socketCallOrIndirect(socket) and
-    var.getAnAssignedValue() = socket and
-    e = var.getAnAccess()
-  )
-}
-
-private predicate socketOutput(FunctionCall call, Expr data) {
-  (
-    // ssize_t send(int sockfd, const void *buf, size_t len, int flags);
-    // ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
-    //                const struct sockaddr *dest_addr, socklen_t addrlen);
-    // ssize_t sendmsg(int sockfd, const struct msghdr *msg, int flags);
-    // int write(int handle, void *buffer, int nbyte);
-    call.getTarget().hasGlobalName(["send", "sendto", "sendmsg", "write"]) and
-    data = call.getArgument(1) and
-    socketFileDescriptor(call.getArgument(0))
-  )
-}
-
-/**
- * Data that is output via a socket.
- */
-class SocketOutput extends DataOutput {
-  SocketOutput() { socketOutput(this, _) }
-
-  override Expr getASource() { socketOutput(this, result) }
-}
-
-from SystemData sd, DataOutput ow
 where
-  sd.getAnExprIndirect() = ow.getASource() or
+  config.hasFlowPath(source, sink) and
-  sd.getAnExprIndirect() = ow.getASource().getAChild*()
+  not exists(
-select ow, "This operation exposes system data from $@.", sd, sd.toString()
+    DataFlow::Node alt // remove duplicate results on conversions
+  |
+    config.hasFlow(source.getNode(), alt) and
+    alt.asConvertedExpr() = sink.getNode().asExpr() and
+    alt != sink.getNode()
+  )
+select sink, source, sink, "This operation exposes system data from $@.", source,
+  source.getNode().toString()

cpp/ql/src/change-notes/2022-02-10-system-data-exposure.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cpp/system-data-exposure` query has been modernized and has converted to a `path-problem` query. There are now fewer false positive results.

cpp/ql/src/change-notes/2022-02-16-insufficient-key-size.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* A new `cpp/insufficient-key-size` query has been added to the default query suite for C/C++. The query finds uses of certain cryptographic algorithms where the key size is too small to provide adequate encryption strength.

cpp/ql/src/change-notes/2022-02-22-using-expired-stack-address.md

@@ -0,0 +1,6 @@
+---
+category: newQuery
+---
+
+- A new query titled "Use of expired stack-address" (`cpp/using-expired-stack-address`) has been added.
+  This query finds accesses to expired stack-allocated memory that escaped via a global variable.

cpp/ql/src/change-notes/2022-02-24-non-https-url.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The "Failure to use HTTPS URLs" (`cpp/non-https-url`) has been improved reducing false positive results, and its precision has been increased to 'high'.

cpp/ql/src/change-notes/2022-03-08-code-duplication-deprecation.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The predicates and classes in the `CodeDuplication` library have been deprecated.

cpp/ql/src/change-notes/2022-03-08-code-duplication-queries.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* The deprecated queries `cpp/duplicate-block`, `cpp/duplicate-function`, `cpp/duplicate-class`, `cpp/duplicate-file`, `cpp/mostly-duplicate-function`,`cpp/similar-file`, `cpp/duplicated-lines-in-files` have been removed.

cpp/ql/src/change-notes/released/0.0.10.md

@@ -0,0 +1,5 @@
+## 0.0.10
+
+### Deprecated Classes
+
+* The `CodeDuplication.Copy`, `CodeDuplication.DuplicateBlock`, and `CodeDuplication.SimilarBlock` classes have been deprecated.

cpp/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.0.9
+lastReleaseVersion: 0.0.10

cpp/ql/src/codeql-suites/exclude-slow-queries.yml

@@ -2,7 +2,6 @@
 # These queries are infeasible to compute on large projects:
 - exclude:
     query path:
-      - Security/CWE/CWE-497/ExposedSystemData.ql
       - Critical/DescriptorMayNotBeClosed.ql
       - Critical/DescriptorNeverClosed.ql
       - Critical/FileMayNotBeClosed.ql

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

@@ -0,0 +1,5 @@
+...
+  vUnsignedLong = (unsigned long)(vUnsignedInt*vUnsignedInt); // BAD
+...
+  vUnsignedLong = ((unsigned long)vUnsignedInt*vUnsignedInt); // GOOD
+...

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

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Search for places where the result of the multiplication is subjected to explicit conversion, not the arguments. Therefore, during the multiplication period, you can lose meaningful data.</p>
+
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed methods for working with type conversion.</p>
+<sample src="DangerousUseOfTransformationAfterOperation.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap">INT30-C. Ensure that unsigned integer operations do not wrap</a>.
+</li>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,102 @@
+/**
+ * @name Dangerous use of transformation after operation.
+ * @description By using the transformation after the operation, you are doing a pointless and dangerous action.
+ * @kind problem
+ * @id cpp/dangerous-use-of-transformation-after-operation
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-190
+ */
+
+import cpp
+
+/** Returns the number of the expression in the function call arguments. */
+int argumentPosition(FunctionCall fc, Expr exp, int n) {
+  n in [0 .. fc.getNumberOfArguments() - 1] and
+  fc.getArgument(n) = exp and
+  result = n
+}
+
+/** Holds if a nonsensical type conversion situation is found. */
+predicate conversionDoneLate(MulExpr mexp) {
+  exists(Expr e1, Expr e2 |
+    mexp.hasOperands(e1, e2) and
+    not e1.isConstant() and
+    not e1.hasConversion() and
+    not e1.hasConversion() and
+    (
+      e2.isConstant() or
+      not e2.hasConversion()
+    ) and
+    mexp.getConversion().hasExplicitConversion() and
+    mexp.getConversion() instanceof ParenthesisExpr and
+    mexp.getConversion().getConversion() instanceof CStyleCast and
+    mexp.getConversion().getConversion().getType().getSize() > mexp.getType().getSize() and
+    mexp.getConversion().getConversion().getType().getSize() > e2.getType().getSize() and
+    mexp.getConversion().getConversion().getType().getSize() > e1.getType().getSize() and
+    exists(Expr e0 |
+      e0.(AssignExpr).getRValue() = mexp.getParent*() and
+      e0.(AssignExpr).getLValue().getType().getSize() =
+        mexp.getConversion().getConversion().getType().getSize()
+      or
+      mexp.getEnclosingElement().(ComparisonOperation).hasOperands(mexp, e0) and
+      e0.getType().getSize() = mexp.getConversion().getConversion().getType().getSize()
+      or
+      e0.(FunctionCall)
+          .getTarget()
+          .getParameter(argumentPosition(e0.(FunctionCall), mexp, _))
+          .getType()
+          .getSize() = mexp.getConversion().getConversion().getType().getSize()
+    )
+  )
+}
+
+/** Holds if the situation of a possible signed overflow used in pointer arithmetic is found. */
+predicate signSmallerWithEqualSizes(MulExpr mexp) {
+  exists(Expr e1, Expr e2 |
+    mexp.hasOperands(e1, e2) and
+    not e1.isConstant() and
+    not e1.hasConversion() and
+    not e1.hasConversion() and
+    (
+      e2.isConstant() or
+      not e2.hasConversion()
+    ) and
+    mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
+    (
+      e2.isConstant() or
+      mexp.getConversion+().getUnderlyingType().getSize() = e2.getUnderlyingType().getSize()
+    ) and
+    mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
+    exists(AssignExpr ae |
+      ae.getRValue() = mexp.getParent*() and
+      ae.getRValue().getUnderlyingType().(IntegralType).isUnsigned() and
+      ae.getLValue().getUnderlyingType().(IntegralType).isSigned() and
+      (
+        not exists(DivExpr de | mexp.getParent*() = de)
+        or
+        exists(DivExpr de, Expr ec |
+          e2.isConstant() and
+          de.hasOperands(mexp.getParent*(), ec) and
+          ec.isConstant() and
+          e2.getValue().toInt() > ec.getValue().toInt()
+        )
+      ) and
+      exists(PointerAddExpr pa |
+        ae.getASuccessor+() = pa and
+        pa.getAnOperand().(VariableAccess).getTarget() = ae.getLValue().(VariableAccess).getTarget()
+      )
+    )
+  )
+}
+
+from MulExpr mexp, string msg
+where
+  conversionDoneLate(mexp) and
+  msg = "This transformation is applied after multiplication."
+  or
+  signSmallerWithEqualSizes(mexp) and
+  msg = "Possible signed overflow followed by offset of the pointer out of bounds."
+select mexp, msg

cpp/ql/src/experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.cpp

@@ -0,0 +1,10 @@
+...
+  FILE *fp = fopen(filename,"w"); // BAD
+...
+  umask(S_IXUSR|S_IRWXG|S_IRWXO);
+  FILE *fp;
+  fp = fopen(filename,"w"); // GOOD
+  chmod(filename,S_IRUSR|S_IWUSR);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+...

cpp/ql/src/experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>When creating a file using a library function such as <code>fopen</code>, the access rights for the newly created file are not specified as part of the call. Instead these rights are determined by the system unless the programmer takes specific measures, such as calling the Posix <code>umask</code> function at some point before the call to <code>fopen</code>. For some applications, the default access rights assigned by the system are not sufficient to protect a file against access by an attacker.</p>
+
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed methods for working with files.</p>
+<sample src="ExposureSensitiveInformationUnauthorizedActor.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/FIO06-C.+Create+files+with+appropriate+access+permissions">FIO06-C. Create files with appropriate access permissions</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.ql

@@ -0,0 +1,65 @@
+/**
+ * @name Writing to a file without setting permissions.
+ * @description Lack of restriction on file access rights can be unsafe.
+ * @kind problem
+ * @id cpp/work-with-file-without-permissions-rights
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       maintainability
+ *       security
+ *       external/cwe/cwe-200
+ *       external/cwe/cwe-264
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/** Holds for a function `f` that has an argument at index `apos` used to read the file. */
+predicate numberArgumentRead(Function f, int apos) {
+  f.hasGlobalOrStdName("fgets") and apos = 2
+  or
+  f.hasGlobalOrStdName("fread") and apos = 3
+  or
+  f.hasGlobalOrStdName("read") and apos = 0
+  or
+  f.hasGlobalOrStdName("fscanf") and apos = 0
+}
+
+/** Holds for a function `f` that has an argument at index `apos` used to write to file */
+predicate numberArgumentWrite(Function f, int apos) {
+  f.hasGlobalOrStdName("fprintf") and apos = 0
+  or
+  f.hasGlobalOrStdName("fputs") and apos = 1
+  or
+  f.hasGlobalOrStdName("write") and apos = 0
+  or
+  f.hasGlobalOrStdName("fwrite") and apos = 3
+  or
+  f.hasGlobalOrStdName("fflush") and apos = 0
+}
+
+from FunctionCall fc
+where
+  // a file is opened
+  (
+    fc.getTarget().hasGlobalOrStdName("fopen") or
+    fc.getTarget().hasGlobalOrStdName("open")
+  ) and
+  fc.getNumberOfArguments() = 2 and
+  // the file is used for writing (but not reading)
+  exists(FunctionCall fctmp, int i |
+    numberArgumentWrite(fctmp.getTarget(), i) and
+    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
+  ) and
+  not exists(FunctionCall fctmp, int i |
+    numberArgumentRead(fctmp.getTarget(), i) and
+    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
+  ) and
+  // a file creation mode is not set globally by `umask` anywhere in the program
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("umask") or
+    fctmp.getTarget().hasGlobalOrStdName("fchmod") or
+    fctmp.getTarget().hasGlobalOrStdName("chmod")
+  )
+select fc, "You may have forgotten to restrict access rights when working with a file."

cpp/ql/src/external/CodeDuplication.qll

@@ -2,59 +2,40 @@
 
 import cpp
 
-private string relativePath(File file) { result = file.getRelativePath().replaceAll("\\", "/") }
+deprecated private newtype TDuplicationOrSimilarity = MKDuplicationOrSimilarity()
-
-cached
-private predicate tokenLocation(string path, int sl, int sc, int ec, int el, Copy copy, int index) {
-  path = copy.sourceFile().getAbsolutePath() and
-  tokens(copy, index, sl, sc, ec, el)
-}
-
-/** A token block used for detection of duplicate and similar code. */
-class Copy extends @duplication_or_similarity {
-  /** Gets the index of the last token in this block. */
-  private int lastToken() { result = max(int i | tokens(this, i, _, _, _, _) | i) }
 
+/**
+ * DEPRECATED: This class is no longer used.
+ *
+ * A token block used for detection of duplicate and similar code.
+ */
+deprecated class Copy extends TDuplicationOrSimilarity {
   /** Gets the index of the token in this block starting at the location `loc`, if any. */
-  int tokenStartingAt(Location loc) {
+  int tokenStartingAt(Location loc) { none() }
-    exists(string filepath, int startline, int startcol |
-      loc.hasLocationInfo(filepath, startline, startcol, _, _) and
-      tokenLocation(filepath, startline, startcol, _, _, this, result)
-    )
-  }
 
   /** Gets the index of the token in this block ending at the location `loc`, if any. */
-  int tokenEndingAt(Location loc) {
+  int tokenEndingAt(Location loc) { none() }
-    exists(string filepath, int endline, int endcol |
-      loc.hasLocationInfo(filepath, _, _, endline, endcol) and
-      tokenLocation(filepath, _, _, endline, endcol, this, result)
-    )
-  }
 
   /** Gets the line on which the first token in this block starts. */
-  int sourceStartLine() { tokens(this, 0, result, _, _, _) }
+  int sourceStartLine() { none() }
 
   /** Gets the column on which the first token in this block starts. */
-  int sourceStartColumn() { tokens(this, 0, _, result, _, _) }
+  int sourceStartColumn() { none() }
 
   /** Gets the line on which the last token in this block ends. */
-  int sourceEndLine() { tokens(this, this.lastToken(), _, _, result, _) }
+  int sourceEndLine() { none() }
 
   /** Gets the column on which the last token in this block ends. */
-  int sourceEndColumn() { tokens(this, this.lastToken(), _, _, _, result) }
+  int sourceEndColumn() { none() }
 
   /** Gets the number of lines containing at least (part of) one token in this block. */
   int sourceLines() { result = this.sourceEndLine() + 1 - this.sourceStartLine() }
 
   /** Gets an opaque identifier for the equivalence class of this block. */
-  int getEquivalenceClass() { duplicateCode(this, _, result) or similarCode(this, _, result) }
+  int getEquivalenceClass() { none() }
 
   /** Gets the source file in which this block appears. */
-  File sourceFile() {
+  File sourceFile() { none() }
-    exists(string name | duplicateCode(this, name, _) or similarCode(this, name, _) |
-      name.replaceAll("\\", "/") = relativePath(result)
-    )
-  }
 
   /**
    * Holds if this element is at the specified location.
@@ -77,29 +58,45 @@ class Copy extends @duplication_or_similarity {
   string toString() { none() }
 }
 
-/** A block of duplicated code. */
+/**
-class DuplicateBlock extends Copy, @duplication {
+ * DEPRECATED: This class is no longer used.
+ *
+ * A block of duplicated code.
+ */
+deprecated class DuplicateBlock extends Copy {
   override string toString() {
     result = "Duplicate code: " + this.sourceLines() + " duplicated lines."
   }
 }
 
-/** A block of similar code. */
+/**
-class SimilarBlock extends Copy, @similarity {
+ * DEPRECATED: This class is no longer used.
+ *
+ * A block of similar code.
+ */
+deprecated class SimilarBlock extends Copy {
   override string toString() {
     result = "Similar code: " + this.sourceLines() + " almost duplicated lines."
   }
 }
 
-/** Gets a function with a body and a location. */
+/**
-FunctionDeclarationEntry sourceMethod() {
+ * DEPRECATED: The `CodeDuplication` library will be removed in a future release.
+ *
+ * Gets a function with a body and a location.
+ */
+deprecated FunctionDeclarationEntry sourceMethod() {
   result.isDefinition() and
   exists(result.getLocation()) and
   numlines(unresolveElement(result.getFunction()), _, _, _)
 }
 
-/** Gets the number of member functions in `c` with a body and a location. */
+/**
-int numberOfSourceMethods(Class c) {
+ * DEPRECATED: The `CodeDuplication` library will be removed in a future release.
+ *
+ * Gets the number of member functions in `c` with a body and a location.
+ */
+deprecated int numberOfSourceMethods(Class c) {
   result =
     count(FunctionDeclarationEntry m |
       m = sourceMethod() and
@@ -107,7 +104,7 @@ int numberOfSourceMethods(Class c) {
     )
 }
 
-private predicate blockCoversStatement(int equivClass, int first, int last, Stmt stmt) {
+deprecated private predicate blockCoversStatement(int equivClass, int first, int last, Stmt stmt) {
   exists(DuplicateBlock b, Location loc |
     stmt.getLocation() = loc and
     first = b.tokenStartingAt(loc) and
@@ -116,13 +113,13 @@ private predicate blockCoversStatement(int equivClass, int first, int last, Stmt
   )
 }
 
-private Stmt statementInMethod(FunctionDeclarationEntry m) {
+deprecated private Stmt statementInMethod(FunctionDeclarationEntry m) {
   result.getParent+() = m.getBlock() and
   not result.getLocation() instanceof UnknownStmtLocation and
   not result instanceof BlockStmt
 }
 
-private predicate duplicateStatement(
+deprecated private predicate duplicateStatement(
   FunctionDeclarationEntry m1, FunctionDeclarationEntry m2, Stmt s1, Stmt s2
 ) {
   exists(int equivClass, int first, int last |
@@ -136,31 +133,39 @@ private predicate duplicateStatement(
 }
 
 /**
+ * DEPRECATED: Information on duplicated statements is no longer available.
+ *
  * Holds if `m1` is a function with `total` lines, and `m2` is a function
  * that has `duplicate` lines in common with `m1`.
  */
-predicate duplicateStatements(
+deprecated predicate duplicateStatements(
   FunctionDeclarationEntry m1, FunctionDeclarationEntry m2, int duplicate, int total
 ) {
   duplicate = strictcount(Stmt s | duplicateStatement(m1, m2, s, _)) and
   total = strictcount(statementInMethod(m1))
 }
 
-/** Holds if `m` and other are identical functions. */
+/**
-predicate duplicateMethod(FunctionDeclarationEntry m, FunctionDeclarationEntry other) {
+ * DEPRECATED: Information on duplicated methods is no longer available.
+ *
+ *  Holds if `m` and other are identical functions.
+ */
+deprecated predicate duplicateMethod(FunctionDeclarationEntry m, FunctionDeclarationEntry other) {
   exists(int total | duplicateStatements(m, other, total, total))
 }
 
 /**
+ * DEPRECATED: Information on similar lines is no longer available.
+ *
  * INTERNAL: do not use.
  *
  * Holds if `line` in `f` is similar to a line somewhere else.
  */
-predicate similarLines(File f, int line) {
+deprecated predicate similarLines(File f, int line) {
   exists(SimilarBlock b | b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()])
 }
 
-private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, File f) {
+deprecated private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, File f) {
   lines =
     strictsum(SimilarBlock b, int toSum |
       (b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
@@ -170,7 +175,7 @@ private predicate similarLinesPerEquivalenceClass(int equivClass, int lines, Fil
     )
 }
 
-private predicate similarLinesCoveredFiles(File f, File otherFile) {
+deprecated private predicate similarLinesCoveredFiles(File f, File otherFile) {
   exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
     exists(int coveredApprox |
       coveredApprox =
@@ -186,8 +191,12 @@ private predicate similarLinesCoveredFiles(File f, File otherFile) {
   )
 }
 
-/** Holds if `coveredLines` lines of `f` are similar to lines in `otherFile`. */
+/**
-predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
+ * DEPRECATED: Information on similar lines is no longer available.
+ *
+ * Holds if `coveredLines` lines of `f` are similar to lines in `otherFile`.
+ */
+deprecated predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
   exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
     similarLinesCoveredFiles(f, otherFile) and
     exists(int notCovered |
@@ -202,17 +211,19 @@ predicate similarLinesCovered(File f, int coveredLines, File otherFile) {
 }
 
 /**
+ * DEPRECATED: Information on duplicate lines is no longer available.
+ *
  * INTERNAL: do not use.
  *
  * Holds if `line` in `f` is duplicated by a line somewhere else.
  */
-predicate duplicateLines(File f, int line) {
+deprecated predicate duplicateLines(File f, int line) {
   exists(DuplicateBlock b |
     b.sourceFile() = f and line in [b.sourceStartLine() .. b.sourceEndLine()]
   )
 }
 
-private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, File f) {
+deprecated private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, File f) {
   lines =
     strictsum(DuplicateBlock b, int toSum |
       (b.sourceFile() = f and b.getEquivalenceClass() = equivClass) and
@@ -222,8 +233,12 @@ private predicate duplicateLinesPerEquivalenceClass(int equivClass, int lines, F
     )
 }
 
-/** Holds if `coveredLines` lines of `f` are duplicates of lines in `otherFile`. */
+/**
-predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
+ * DEPRECATED: Information on duplicate lines is no longer available.
+ *
+ *  Holds if `coveredLines` lines of `f` are duplicates of lines in `otherFile`.
+ */
+deprecated predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
   exists(int numLines | numLines = f.getMetrics().getNumberOfLines() |
     exists(int coveredApprox |
       coveredApprox =
@@ -247,8 +262,12 @@ predicate duplicateLinesCovered(File f, int coveredLines, File otherFile) {
   )
 }
 
-/** Holds if most of `f` (`percent`%) is similar to `other`. */
+/**
-predicate similarFiles(File f, File other, int percent) {
+ * DEPRECATED: Information on similar files is no longer available.
+ *
+ * Holds if most of `f` (`percent`%) is similar to `other`.
+ */
+deprecated predicate similarFiles(File f, File other, int percent) {
   exists(int covered, int total |
     similarLinesCovered(f, covered, other) and
     total = f.getMetrics().getNumberOfLines() and
@@ -258,8 +277,12 @@ predicate similarFiles(File f, File other, int percent) {
   not duplicateFiles(f, other, _)
 }
 
-/** Holds if most of `f` (`percent`%) is duplicated by `other`. */
+/**
-predicate duplicateFiles(File f, File other, int percent) {
+ * DEPRECATED: Information on duplicate files is no longer available.
+ *
+ *  Holds if most of `f` (`percent`%) is duplicated by `other`.
+ */
+deprecated predicate duplicateFiles(File f, File other, int percent) {
   exists(int covered, int total |
     duplicateLinesCovered(f, covered, other) and
     total = f.getMetrics().getNumberOfLines() and
@@ -269,10 +292,12 @@ predicate duplicateFiles(File f, File other, int percent) {
 }
 
 /**
+ * DEPRECATED: Information on duplciate classes is no longer available.
+ *
  * Holds if most member functions of `c` (`numDup` out of `total`) are
  * duplicates of member functions in `other`.
  */
-predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
+deprecated predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total) {
   numDup =
     strictcount(FunctionDeclarationEntry m1 |
       exists(FunctionDeclarationEntry m2 |
@@ -288,11 +313,13 @@ predicate mostlyDuplicateClassBase(Class c, Class other, int numDup, int total)
 }
 
 /**
+ * DEPRECATED: Information on duplciate classes is no longer available.
+ *
  * Holds if most member functions of `c` are duplicates of member functions in
  * `other`. Provides the human-readable `message` to describe the amount of
  * duplication.
  */
-predicate mostlyDuplicateClass(Class c, Class other, string message) {
+deprecated predicate mostlyDuplicateClass(Class c, Class other, string message) {
   exists(int numDup, int total |
     mostlyDuplicateClassBase(c, other, numDup, total) and
     (
@@ -316,21 +343,31 @@ predicate mostlyDuplicateClass(Class c, Class other, string message) {
   )
 }
 
-/** Holds if `f` and `other` are similar or duplicates. */
+/**
-predicate fileLevelDuplication(File f, File other) {
+ * DEPRECATED: Information on file duplication is no longer available.
+ *
+ * Holds if `f` and `other` are similar or duplicates.
+ */
+deprecated predicate fileLevelDuplication(File f, File other) {
   similarFiles(f, other, _) or duplicateFiles(f, other, _)
 }
 
 /**
+ * DEPRECATED: Information on class duplication is no longer available.
+ *
  * Holds if most member functions of `c` are duplicates of member functions in
  * `other`.
  */
-predicate classLevelDuplication(Class c, Class other) { mostlyDuplicateClass(c, other, _) }
+deprecated predicate classLevelDuplication(Class c, Class other) {
+  mostlyDuplicateClass(c, other, _)
+}
 
 /**
+ * DEPRECATED: The CodeDuplication library will be removed in a future release.
+ *
  * Holds if `line` in `f` should be allowed to be duplicated. This is the case
  * for `#include` directives.
  */
-predicate whitelistedLineForDuplication(File f, int line) {
+deprecated predicate whitelistedLineForDuplication(File f, int line) {
   exists(Include i | i.getFile() = f and i.getLocation().getStartLine() = line)
 }

cpp/ql/src/external/DuplicateBlock.ql

@@ -1,27 +0,0 @@
-/**
- * @deprecated
- * @name Duplicate code
- * @description This block of code is duplicated elsewhere. If possible, the shared code should be refactored so there is only one occurrence left. It may not always be possible to address these issues; other duplicate code checks (such as duplicate function, duplicate class) give subsets of the results with higher confidence.
- * @kind problem
- * @id cpp/duplicate-block
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       maintainability
- *       duplicate-code
- *       non-attributable
- */
-
-import CodeDuplication
-
-from DuplicateBlock d, DuplicateBlock other, int lines, File otherFile, int otherLine
-where
-  lines = d.sourceLines() and
-  lines > 10 and
-  other.getEquivalenceClass() = d.getEquivalenceClass() and
-  other != d and
-  otherFile = other.sourceFile() and
-  otherLine = other.sourceStartLine()
-select d,
-  "Duplicate code: " + lines + " lines are duplicated at " + otherFile.getBaseName() + ":" +
-    otherLine

cpp/ql/src/external/DuplicateFunction.qhelp

@@ -1,43 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>A function should never be duplicated verbatim in several places in the code. Of course
-the severity of this anti-pattern is higher for longer functions than for extremely short
-functions of one or two statements, but there are usually better ways of achieving the same
-effect.</p>
-
-</overview>
-<recommendation>
-<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
-inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
-can mask the real purpose or intention behind a function. There is also a risk of
-update anomalies, where only one of several copies of the code is updated to address a defect or
-add a feature.</p>
-
-<p>In the case of function duplication, how to address the issue depends on the functions themselves
-and on the precise classes in which the duplication occurs. At its simplest, the duplication can
-be addressed by removing all but one of the duplicate function definitions and making
-callers of the removed functions refer to the (now canonical) single remaining definition
-instead.</p>
-
-<p>This may not be possible for reasons of visibility or accessibility. A common example might
-be where two classes implement the same functionality but neither is a subtype of the other,
-so it is not possible to inherit a single function definition. In such cases, introducing a
-common superclass to share the duplicated code is a viable option. Alternatively, if the functions
-don't need access to private object state, they can be moved to a shared utility class that
-just provides the functionality itself.</p>
-
-</recommendation>
-<references>
-
-<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009. 
-Do code clones matter?  In <em>Proceedings of the 31st International Conference on
-Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA, 
-485-495.</li>
-
-</references>
-</qhelp>

cpp/ql/src/external/DuplicateFunction.ql

@@ -1,39 +0,0 @@
-/**
- * @deprecated
- * @name Duplicate function
- * @description There is another identical implementation of this function. Extract the code to a common file or superclass or delegate to improve sharing.
- * @kind problem
- * @id cpp/duplicate-function
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       maintainability
- *       duplicate-code
- *       non-attributable
- */
-
-import cpp
-import CodeDuplication
-
-predicate relevant(FunctionDeclarationEntry m) {
-  exists(Location loc |
-    loc = m.getBlock().getLocation() and
-    (
-      loc.getStartLine() + 5 < loc.getEndLine() and not m.getName().matches("get%")
-      or
-      loc.getStartLine() + 10 < loc.getEndLine()
-    )
-  )
-}
-
-from FunctionDeclarationEntry m, FunctionDeclarationEntry other
-where
-  duplicateMethod(m, other) and
-  relevant(m) and
-  not m.getFunction().isConstructedFrom(_) and
-  not other.getFunction().isConstructedFrom(_) and
-  not fileLevelDuplication(m.getFile(), other.getFile()) and
-  not classLevelDuplication(m.getFunction().getDeclaringType(),
-    other.getFunction().getDeclaringType())
-select m, "Function " + m.getName() + " is duplicated at $@.", other,
-  other.getFile().getBaseName() + ":" + other.getLocation().getStartLine().toString()

cpp/ql/src/external/MostlyDuplicateClass.qhelp

@@ -1,45 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>In cases where several methods are duplicated between two (or more) classes,
-the classes themselves are highlighted as "mostly duplicate", rather than creating a large
-number of method-level warnings. The same caveats apply here, too.</p>
-
-</overview>
-<recommendation>
-<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
-inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
-can mask the real purpose or intention behind a method. There's also an omnipresent risk of
-update anomalies, where only one of several copies of the code is updated to address a defect or
-add a feature.</p>
-
-<p>While completely duplicated classes are rare, they are usually a sign of a simple
-oversight (or deliberate copy/paste) on a developer's part. Usually the required remedy
-action is to remove all but one of them.</p>
-
-<p>It is far more common to see duplication of many methods between two classes, leaving just
-a few that are actually different. Such situations warrant close inspection. Are the differences
-deliberate or a result of an inconsistent update to one of the clones? If the latter, then
-treating the classes as completely duplicate and eliminating all but one (while preserving any
-corrections or new features that may have been introduced) is the best course. If two classes serve
-genuinely different purposes but almost all of their methods are the same, that can be a sign
-that there is a missing level of abstraction. Introducing a common superclass to define the
-common methods and sharing the code is likely to prevent many problems in the long term.</p>
-
-<p>Modern IDEs may provide refactoring support for this sort of transformation, usually
-under the names of "Pull up" or "Extract supertype".</p>
-
-</recommendation>
-<references>
-
-<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009. 
-Do code clones matter?  In <em>Proceedings of the 31st International Conference on
-Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA, 
-485-495.</li>
-
-</references>
-</qhelp>

cpp/ql/src/external/MostlyDuplicateClass.ql

@@ -1,24 +0,0 @@
-/**
- * @deprecated
- * @name Mostly duplicate class
- * @description More than 80% of the methods in this class are duplicated in another class. Create a common supertype to improve code sharing.
- * @kind problem
- * @id cpp/duplicate-class
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       maintainability
- *       duplicate-code
- *       non-attributable
- */
-
-import cpp
-import CodeDuplication
-
-from Class c, Class other, string message
-where
-  mostlyDuplicateClass(c, other, message) and
-  not c.isConstructedFrom(_) and
-  not other.isConstructedFrom(_) and
-  not fileLevelDuplication(c.getFile(), _)
-select c, message, other, other.getQualifiedName()

cpp/ql/src/external/MostlyDuplicateFile.qhelp

@@ -1,46 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>In cases where most of a file's lines have been duplicated in one or more other
-files, the files themselves are highlighted as "mostly duplicate", rather than
-creating a large number of method-level or class-level warnings. The same caveats
-apply here, too.</p>
-
-</overview>
-<recommendation>
-<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
-inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
-can mask the real purpose or intention behind a method. There's also an omnipresent risk of
-update anomalies, where only one of several copies of the code is updated to address a defect or
-add a feature.</p>
-
-<p>While completely duplicated files are rare, they are usually a sign of a simple
-oversight (or deliberate copy/paste) on a developer's part. Usually the required remedy
-action is to remove all but one of them. A common exception may arise from generated code
-that simply occurs in several places in the source tree; the check can be adapted to
-exclude such results.</p>
-
-<p>It is far more common to see duplication of many lines between two files, leaving just
-a few that are actually different. Such situations warrant close inspection. Are the differences
-deliberate or a result of an inconsistent update to one of the clones? If the latter, then
-treating the files as completely duplicate and eliminating all but one (while preserving any
-corrections or new features that may have been introduced) is the best course. If two files serve
-genuinely different purposes but almost all of their lines are the same, that can be a sign
-that there is a missing level of abstraction. Look for ways to share the functionality, either
-by extracting a utility class for parts of it or by encapsulating the common parts into a new
-super class of any classes involved.</p>
-
-</recommendation>
-<references>
-
-<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009. 
-Do code clones matter?  In <em>Proceedings of the 31st International Conference on
-Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA, 
-485-495.</li>
-
-</references>
-</qhelp>

cpp/ql/src/external/MostlyDuplicateFile.ql

@@ -1,21 +0,0 @@
-/**
- * @deprecated
- * @name Mostly duplicate file
- * @description There is another file that shares a lot of the code with this file. Merge the two files to improve maintainability.
- * @kind problem
- * @id cpp/duplicate-file
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       maintainability
- *       duplicate-code
- *       non-attributable
- */
-
-import cpp
-import CodeDuplication
-
-from File f, File other, int percent
-where duplicateFiles(f, other, percent)
-select f, percent + "% of the lines in " + f.getBaseName() + " are copies of lines in $@.", other,
-  other.getBaseName()

cpp/ql/src/external/MostlyDuplicateFunction.qhelp

@@ -1,55 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-<overview>
-<p>A "mostly duplicate function" is a function for which there is at least one
-almost exact duplicate somewhere else in the code, but for which there are no
-exact duplicates. There will be minor typographical differences between this
-function and any "mostly duplicate function" to which it corresponds (for
-example, comments and small code changes), preventing an exact match. Pairs of
-such functions are sometimes referred to as "similar".</p>
-
-<p>This class of problem can often be more insidious than mere duplication, because the two
-implementations have diverged. This may be on purpose (when a function is copy-and-pasted
-and adapted to a new context) or accidentally (when a correction is only introduced in one of
-several identical pieces of code), and to address the problem one needs to understand which
-of the two situations applies.</p>
-
-</overview>
-<recommendation>
-<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
-inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
-can mask the real purpose or intention behind a function. There's also a risk of
-update anomalies, where only one of several copies of the code is updated to address a defect or
-add a feature.</p>
-
-<p>In the case of function similarity, how to address the issue depends on the functions
-themselves and on the precise classes in which they occur. At its simplest, if the differences
-are accidental, the problem can be addressed by unifying the functions to behave identically.
-Then, we can remove all but one of the duplicate function definitions and make
-callers of the removed functions refer to the (now canonical) single remaining definition
-instead.</p>
-
-<p>In more complex cases, look for ways of encapsulating the commonality and sharing it while
-retaining the differences in functionality. Perhaps the function can be moved to a single place
-and given an additional parameter, allowing it to cover all use cases? Alternatively, there
-may be a common preprocessing or postprocessing step which can be extracted to its own (shared)
-function, leaving only the specific parts in the existing functions.</p>
-
-<p>Modern IDEs may provide refactoring support for this sort of transformation. Relevant
-refactorings might be "Extract function", "Change function signature", "Pull up" or "Extract
-supertype".</p>
-
-</recommendation>
-<references>
-
-<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009. 
-Do code clones matter?  In <em>Proceedings of the 31st International Conference on
-Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA, 
-485-495.</li>
-
-
-</references>
-</qhelp>

cpp/ql/src/external/MostlyDuplicateFunction.ql

@@ -1,31 +0,0 @@
-/**
- * @deprecated
- * @name Mostly duplicate function
- * @description There is another function that shares a lot of the code with this one. Extract the code to a common file/superclass or delegate to improve sharing.
- * @kind problem
- * @id cpp/mostly-duplicate-function
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       duplicate-code
- *       non-attributable
- */
-
-import cpp
-import CodeDuplication
-
-from FunctionDeclarationEntry m, int covered, int total, FunctionDeclarationEntry other, int percent
-where
-  duplicateStatements(m, other, covered, total) and
-  covered != total and
-  total > 5 and
-  covered * 100 / total = percent and
-  percent > 80 and
-  not m.getFunction().isConstructedFrom(_) and
-  not other.getFunction().isConstructedFrom(_) and
-  not duplicateMethod(m, other) and
-  not classLevelDuplication(m.getFunction().getDeclaringType(),
-    other.getFunction().getDeclaringType()) and
-  not fileLevelDuplication(m.getFile(), other.getFile())
-select m, percent + "% of the statements in " + m.getName() + " are duplicated in $@.", other,
-  other.getFunction().getQualifiedName()

cpp/ql/src/external/MostlySimilarFile.qhelp

@@ -1,39 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-
-
-<overview>
-<p>Two lines are defined as similar if they are either identical or contain only very minor
-differences. In cases where most of the lines in a file have corresponding "similar" lines in another file, 
-the files themselves are highlighted as "mostly similar", instead of creating a large number of method-level or class-level
-warnings. The same caveats apply here, too.</p>
-
-</overview>
-<recommendation>
-<p>Code duplication in general is highly undesirable for a range of reasons: The artificially
-inflated amount of code hinders comprehension, and ranges of similar but subtly different lines
-can mask the real purpose or intention behind a method. There's also an omnipresent risk of
-update anomalies, where only one of several copies of the code is updated to address a defect or
-add a feature.</p>
-
-<p>With files that are marked as mostly similar, special care should be taken. Why are
-they almost the same, and why are there differences? If the differences are accidental (for
-example from corrections that were only applied to one copy), then unifying the files and removing
-all but one is the best thing to do. If the files have genuinely different tasks, it is worth
-thinking about the reasons for the similarity. Can some of the shared code be extracted into
-methods (perhaps with additional parameters, to cover the differences in behavior)? Should it
-be moved into a utility class or file that is accessible to all current implementations, or
-should a new level of abstraction be introduced?</p>
-
-</recommendation>
-<references>
-
-<li>Elmar Juergens, Florian Deissenboeck, Benjamin Hummel, and Stefan Wagner. 2009. 
-Do code clones matter?  In <em>Proceedings of the 31st International Conference on
-Software Engineering</em> (ICSE '09). IEEE Computer Society, Washington, DC, USA, 
-485-495.</li>
-
-</references>
-</qhelp>

cpp/ql/src/external/MostlySimilarFile.ql

@@ -1,21 +0,0 @@
-/**
- * @deprecated
- * @name Mostly similar file
- * @description There is another file that shares a lot of the code with this file. Notice that names of variables and types may have been changed. Merge the two files to improve maintainability.
- * @kind problem
- * @id cpp/similar-file
- * @problem.severity recommendation
- * @precision medium
- * @tags testability
- *       maintainability
- *       duplicate-code
- *       non-attributable
- */
-
-import cpp
-import CodeDuplication
-
-from File f, File other, int percent
-where similarFiles(f, other, percent)
-select f, percent + "% of the lines in " + f.getBaseName() + " are similar to lines in $@.", other,
-  other.getBaseName()

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 0.0.9
+version: 0.0.11-dev
 groups: 
   - cpp
   - queries

cpp/ql/test/TestUtilities/InlineExpectationsTest.qll

@@ -124,7 +124,9 @@ abstract class InlineExpectationsTest extends string {
   abstract predicate hasActualResult(Location location, string element, string tag, string value);
 
   /**
-   * Like `hasActualResult`, but returns results that do not require a matching annotation.
+   * Holds if there is an optional result on the specified location.
+   *
+   * This is similar to `hasActualResult`, but returns results that do not require a matching annotation.
    * A failure will still arise if there is an annotation that does not match any results, but not vice versa.
    * Override this predicate to specify optional results.
    */

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.expected

@@ -0,0 +1,4 @@
+| test.cpp:9:8:9:12 | ... * ... | Possible signed overflow followed by offset of the pointer out of bounds. |
+| test.cpp:13:24:13:28 | ... * ... | This transformation is applied after multiplication. |
+| test.cpp:16:28:16:32 | ... * ... | This transformation is applied after multiplication. |
+| test.cpp:19:22:19:26 | ... * ... | This transformation is applied after multiplication. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/test.cpp

@@ -0,0 +1,23 @@
+void testCall (unsigned long);
+void functionWork(char aA[10],unsigned int aUI) {
+
+  unsigned long aL;
+  char *aP;
+  int aI;
+
+  aI = (aUI*8)/10; // GOOD
+  aI = aUI*8; // BAD
+  aP = aA+aI;
+  aI = (int)aUI*8; // GOOD
+  
+  aL = (unsigned long)(aI*aI); // BAD
+  aL = ((unsigned long)aI*aI); // GOOD
+  
+  testCall((unsigned long)(aI*aI)); // BAD
+  testCall(((unsigned long)aI*aI)); // GOOD
+  
+  if((unsigned long)(aI*aI) > aL) // BAD
+    return;
+  if(((unsigned long)aI*aI) > aL) // GOOD
+    return;
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/ExposureSensitiveInformationUnauthorizedActor.expected

@@ -0,0 +1 @@
+| test.cpp:12:8:12:12 | call to fopen | You may have forgotten to restrict access rights when working with a file. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/ExposureSensitiveInformationUnauthorizedActor.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/test.cpp

@@ -0,0 +1,17 @@
+typedef int FILE;
+FILE *fopen(const char *filename, const char *mode);
+int umask(int pmode);
+int chmod(char * filename,int pmode);
+int fprintf(FILE *fp,const char *fmt, ...);
+int fclose(FILE *stream);
+
+int main(int argc, char *argv[])
+{
+  //umask(0022);
+  FILE *fp;
+  fp = fopen("myFile.txt","w"); // BAD
+  //chmod("myFile.txt",0644);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+  return 0;
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test2/ExposureSensitiveInformationUnauthorizedActor.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test2/test.cpp

@@ -0,0 +1,17 @@
+typedef int FILE;
+FILE *fopen(const char *filename, const char *mode);
+int umask(int pmode);
+int chmod(char * filename,int pmode);
+int fprintf(FILE *fp,const char *fmt, ...);
+int fclose(FILE *stream);
+
+int main(int argc, char *argv[])
+{
+  umask(0022);
+  FILE *fp;
+  fp = fopen("myFile.txt","w"); // GOOD
+  chmod("myFile.txt",0644);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+  return 0;
+}