Merge pull request #8397 from github/release-prep/2.8.3

Release preparation for version 2.8.3

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

.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**
 
@@ -63,6 +67,6 @@ After the experimental query is merged, we welcome pull requests to improve it.
 
 ## Using your personal data
 
-If you contribute to this project, we will record your name and email address (as provided by you with your contributions) as part of the code repositories, which are public. We might also use this information to contact you in relation to your contributions, as well as in the normal course of software development. We also store records of CLA agreements signed in the past, but no longer require contributors to sign a CLA. Under GDPR legislation, we do this on the basis of our legitimate interest in creating the CodeQL product. 
+If you contribute to this project, we will record your name and email address (as provided by you with your contributions) as part of the code repositories, which are public. We might also use this information to contact you in relation to your contributions, as well as in the normal course of software development. We also store records of CLA agreements signed in the past, but no longer require contributors to sign a CLA. Under GDPR legislation, we do this on the basis of our legitimate interest in creating the CodeQL product.
 
 Please do get in touch (privacy@github.com) if you have any questions about this or our data protection policies.

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"

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/ql/lib/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.0.11
+
+### Minor Analysis Improvements
+
+* Many queries now support structured bindings, as structured bindings are now handled in the IR translation.
+
 ## 0.0.10
 
 ### New Features
@@ -6,6 +12,7 @@
 
 ## 0.0.9
 
+
 ## 0.0.8
 
 ### Deprecated APIs

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

@@ -0,0 +1,5 @@
+## 0.0.11
+
+### Minor Analysis Improvements
+
+* Many queries now support structured bindings, as structured bindings are now handled in the IR translation.

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

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

cpp/ql/lib/qlpack.yml

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

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
-      result = resolveCall(call).getParameter(i)
+      sink.asExpr() = call.getArgument(pragma[only_bind_into](i)) and
+      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/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
-  strictcount(instr.getIRVariable().getAST()) = 1
+  unique( | | instr.getIRVariable().getAST()) = ast
 }
 
 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
-  instr.getResultIRType() = type and
+  unique( | | instr.getResultIRType()) = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
-  strictcount(instr.getField()) = 1 and
+  unique( | | instr.getField()) = field and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
-  instr.getDerivedClass() = derivedClass and
-  tvalueNumber(instr.getUnary()) = operand
+  tvalueNumber(instr.getUnary()) = operand and
+  unique( | | instr.getBaseClass()) = baseClass and
+  unique( | | instr.getDerivedClass()) = derivedClass
 }
 
 private predicate loadTotalOverlapValueNumber(

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
-  strictcount(instr.getIRVariable().getAST()) = 1
+  unique( | | instr.getIRVariable().getAST()) = ast
 }
 
 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
-  instr.getResultIRType() = type and
+  unique( | | instr.getResultIRType()) = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
-  strictcount(instr.getField()) = 1 and
+  unique( | | instr.getField()) = field and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
-  instr.getDerivedClass() = derivedClass and
-  tvalueNumber(instr.getUnary()) = operand
+  tvalueNumber(instr.getUnary()) = operand and
+  unique( | | instr.getBaseClass()) = baseClass and
+  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/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
-  strictcount(instr.getIRVariable().getAST()) = 1
+  unique( | | instr.getIRVariable().getAST()) = ast
 }
 
 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
-  instr.getResultIRType() = type and
+  unique( | | instr.getResultIRType()) = type and
   instr.getValue() = value
 }
 
@@ -151,8 +155,7 @@ private predicate fieldAddressValueNumber(
   TValueNumber objectAddress
 ) {
   instr.getEnclosingIRFunction() = irFunc and
-  instr.getField() = field and
-  strictcount(instr.getField()) = 1 and
+  unique( | | instr.getField()) = field and
   tvalueNumber(instr.getObjectAddress()) = objectAddress
 }
 
@@ -195,9 +198,9 @@ private predicate inheritanceConversionValueNumber(
 ) {
   instr.getEnclosingIRFunction() = irFunc and
   instr.getOpcode() = opcode and
-  instr.getBaseClass() = baseClass and
-  instr.getDerivedClass() = derivedClass and
-  tvalueNumber(instr.getUnary()) = operand
+  tvalueNumber(instr.getUnary()) = operand and
+  unique( | | instr.getBaseClass()) = baseClass and
+  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,7 +39,12 @@ Type getVariableType(Variable v) {
         result = v.getInitializer().getExpr().getType()
         or
         not exists(v.getInitializer()) and result = v.getType()
-      else 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/src/CHANGELOG.md

@@ -1,3 +1,24 @@
+## 0.0.11
+
+### Breaking Changes
+
+* 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.
+
+### Deprecated Predicates and Classes
+
+* The predicates and classes in the `CodeDuplication` library have been deprecated.
+
+### New Queries
+
+* 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.
+* 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.
+
+### Minor Analysis Improvements
+
+* 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'.
+* The `cpp/system-data-exposure` query has been modernized and has converted to a `path-problem` query. There are now fewer false positive results.
+
 ## 0.0.10
 
 ### Deprecated Classes

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

@@ -18,157 +18,71 @@ 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%"])
 }
 
-/**
- * Holds if `source` is a node that represents the use of a stack variable
- */
-predicate isSource(Node source) {
-  exists(VariableAddressInstruction var, Function func |
-    var = source.asInstruction() and
-    func = var.getEnclosingFunction() and
-    var.getASTVariable() instanceof StackVariable and
-    // Pointer-to-member types aren't properly handled in the dbscheme.
-    not var.getResultType() instanceof PointerToMemberType and
-    // Rule out FPs caused by extraction errors.
-    not any(ErrorExpr e).getEnclosingFunction() = func and
-    not intentionallyReturnsStackPointer(func)
-  )
-}
+class ReturnStackAllocatedMemoryConfig extends MustFlowConfiguration {
+  ReturnStackAllocatedMemoryConfig() { this = "ReturnStackAllocatedMemoryConfig" }
 
-/**
- * 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
- * `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) {
-  instructionToOperandStep(node1.asInstruction(), node2.asOperand())
-  or
-  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 };
- * int* test() {
- *   S s;
- *   return &s.x; // BAD: &s.x is an address of a variable on the stack.
- * }
- * ```
- */
-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.(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))
+  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
+      var.getASTVariable() instanceof StackVariable and
+      // Pointer-to-member types aren't properly handled in the dbscheme.
+      not var.getResultType() instanceof PointerToMemberType and
+      // Rule out FPs caused by extraction errors.
+      not any(ErrorExpr e).getEnclosingFunction() = func and
+      not intentionallyReturnsStackPointer(func)
     )
   }
 
-abstract class LocalPathNode extends TLocalPathNode {
-  Node n;
+  override predicate isSink(DataFlow::Node sink) {
+    // 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
+    // `ReturnValueInstruction`'s source value oprand because the former has better location information.
+    exists(StoreInstruction store |
+      store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
+        IRReturnVariable and
+      sink.asOperand() = store.getSourceValueOperand()
+    )
+  }
 
-  /** 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()) }
+  /**
+   * This configuration 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 };
+   * int* test() {
+   *   S s;
+   *   return &s.x; // BAD: &s.x is an address of a variable on the stack.
+   * }
+   * ```
+   */
+  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    node2.asInstruction().(FieldAddressInstruction).getObjectAddressOperand() = node1.asOperand()
+    or
+    node2.asInstruction().(PointerOffsetInstruction).getLeftOperand() = node1.asOperand()
+  }
 }
 
-class LocalPathNodeMid extends LocalPathNode, TLocalPathNodeMid {
-  LocalPathNodeMid() { this = TLocalPathNodeMid(n) }
-}
-
-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
+from
+  MustFlowPathNode source, MustFlowPathNode sink, VariableAddressInstruction var,
+  ReturnStackAllocatedMemoryConfig conf
 where
-  hasFlow(source, sink) and
-  source.getNode().asInstruction() = var
+  conf.hasFlowPath(source, sink) and
+  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,169 +17,47 @@
 import cpp
 // We don't actually use the global value numbering library in this query, but without it we end up
 // recomputing the IR.
-private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
-private import semmle.code.cpp.ir.IR
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.MustFlow
+import PathGraph
 
-bindingset[n, result]
-int unbind(int n) { result >= n and result <= n }
+class UnsafeUseOfThisConfig extends MustFlowConfiguration {
+  UnsafeUseOfThisConfig() { this = "UnsafeUseOfThisConfig" }
 
-/** Holds if `p` is the `n`'th parameter of the non-virtual function `f`. */
-predicate parameterOf(Parameter p, Function f, int n) {
-  not f.isVirtual() and f.getParameter(n) = p
-}
+  override predicate isSource(DataFlow::Node source) { isSource(source, _, _) }
 
-/**
- * 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
+  override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
 }
 
 /** 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) {
-  (
-    exists(Constructor func |
-      not func instanceof CopyConstructor and
-      not func instanceof MoveConstructor and
-      func = init.getEnclosingFunction() and
-      msg = "construction"
-    )
-    or
-    init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
-  ) and
-  init.getIRVariable() instanceof IRThisVariable and
-  init.getEnclosingFunction().getDeclaringType() = c
-}
-
-/**
- * Holds if `instr` flows to a sink (which is a use of the value of `instr` as a `this` pointer).
- */
-predicate flowsToSink(Instruction instr, Instruction sink) {
-  flowsFromSource(instr) and
-  (
-    isSink(instr, _) and instr = sink
-    or
-    exists(Instruction mid |
-      successor(instr, mid) and
-      flowsToSink(mid, sink)
-    )
+predicate isSource(DataFlow::Node source, string msg, Class c) {
+  exists(InitializeParameterInstruction init | init = source.asInstruction() |
+    (
+      exists(Constructor func |
+        not func instanceof CopyConstructor and
+        not func instanceof MoveConstructor and
+        func = init.getEnclosingFunction() and
+        msg = "construction"
+      )
+      or
+      init.getEnclosingFunction() instanceof Destructor and msg = "destruction"
+    ) and
+    init.getIRVariable() instanceof IRThisVariable and
+    init.getEnclosingFunction().getDeclaringType() = c
   )
 }
 
-/** Holds if `instr` flows from a source. */
-predicate flowsFromSource(Instruction instr) {
-  isSource(instr, _, _)
-  or
-  exists(Instruction mid |
-    successor(mid, instr) and
-    flowsFromSource(mid)
-  )
-}
-
-/** Holds if `f` is the enclosing non-virtual function of `init`. */
-predicate getEnclosingNonVirtualFunctionInitializeParameter(
-  InitializeParameterInstruction init, Function f
-) {
-  not f.isVirtual() and
-  init.getEnclosingFunction() = f
-}
-
-/** Holds if `f` is the enclosing non-virtual function of `init`. */
-predicate getEnclosingNonVirtualFunctionInitializeIndirection(
-  InitializeIndirectionInstruction init, Function f
-) {
-  not f.isVirtual() and
-  init.getEnclosingFunction() = f
-}
-
-/**
- * Holds if `instr` is an argument (or argument indirection) to a call, and
- * `succ` is the corresponding initialization instruction in the call target.
- */
-predicate flowThroughCallable(Instruction instr, Instruction succ) {
-  // Flow from an argument to a parameter
-  exists(CallInstruction call, InitializeParameterInstruction init | init = succ |
-    getPositionalArgumentInitParam(call, instr, init, call.getStaticCallTarget())
-    or
-    getThisArgumentInitParam(call, instr, init, call.getStaticCallTarget())
-  )
-  or
-  // Flow from argument indirection to parameter indirection
-  exists(
-    CallInstruction call, ReadSideEffectInstruction read, InitializeIndirectionInstruction init
-  |
-    init = succ and
-    read.getPrimaryInstruction() = call and
-    getEnclosingNonVirtualFunctionInitializeIndirection(init, call.getStaticCallTarget())
-  |
-    exists(int n |
-      read.getSideEffectOperand().getAnyDef() = instr and
-      read.getIndex() = n and
-      init.getParameter().getIndex() = unbind(n)
-    )
-    or
-    call.getThisArgument() = instr and
-    init.getIRVariable() instanceof IRThisVariable
-  )
-}
-
-/** Holds if `instr` flows to `succ`. */
-predicate successor(Instruction instr, Instruction succ) {
-  succ.(CopyInstruction).getSourceValue() = instr or
-  succ.(CheckedConvertOrNullInstruction).getUnary() = instr or
-  succ.(ChiInstruction).getTotal() = instr or
-  succ.(ConvertInstruction).getUnary() = instr or
-  succ.(InheritanceConversionInstruction).getUnary() = instr or
-  flowThroughCallable(instr, succ)
-}
-
 /**
  * Holds if:
  * - `source` is an initialization of a `this` pointer of type `sourceClass`, and
@@ -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
-  flowsToSink(source, sink) and
-  isSink(sink, call)
+  exists(UnsafeUseOfThisConfig conf |
+    conf.hasFlowPath(source, sink) and
+    isSource(source.getNode(), msg, sourceClass) and
+    isSink(sink.getNode(), call)
+  )
 }
 
-query predicate edges(Instruction a, Instruction b) { successor(a, b) and flowsToSink(b, _) }
-
-query predicate nodes(Instruction n, string key, string val) {
-  flowsToSink(n, _) and
-  key = "semmle.label" and
-  val = n.toString()
-}
-
-from Instruction source, Instruction sink, CallInstruction call, string msg, Class sourceClass
+from
+  MustFlowPathNode source, MustFlowPathNode 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) }
 }
 
-/**
- * Somewhere data is output.
- */
-abstract class DataOutput extends Element {
-  /**
-   * Get an expression containing data that is output.
-   */
-  abstract Expr getASource();
-}
+class ExposedSystemDataConfiguration extends TaintTracking::Configuration {
+  ExposedSystemDataConfiguration() { this = "ExposedSystemDataConfiguration" }
 
-/**
- * Data that is output via standard output or standard error.
- */
-class StandardOutput extends DataOutput instanceof OutputWrite {
-  override Expr getASource() { result = OutputWrite.super.getASource() }
-}
+  override predicate isSource(DataFlow::Node source) {
+    source.asConvertedExpr() = any(SystemData sd).getAnExpr()
+  }
 
-private predicate socketCallOrIndirect(FunctionCall call) {
-  // direct socket call
-  // int socket(int domain, int type, int protocol);
-  call.getTarget().getName() = "socket"
-  or
-  exists(ReturnStmt rtn |
-    // indirect socket call
-    call.getTarget() = rtn.getEnclosingFunction() and
-    (
-      socketCallOrIndirect(rtn.getExpr()) or
-      socketCallOrIndirect(rtn.getExpr().(VariableAccess).getTarget().getAnAssignedValue())
+  override predicate isSink(DataFlow::Node sink) {
+    exists(FunctionCall fc, FunctionInput input, int arg |
+      fc.getTarget().(RemoteFlowSinkFunction).hasRemoteFlowSink(input, _) and
+      input.isParameterDeref(arg) and
+      fc.getArgument(arg).getAChild*() = sink.asExpr()
     )
-  )
+  }
 }
 
-private predicate socketFileDescriptor(Expr e) {
-  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
+from ExposedSystemDataConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where
-  sd.getAnExprIndirect() = ow.getASource() or
-  sd.getAnExprIndirect() = ow.getASource().getAChild*()
-select ow, "This operation exposes system data from $@.", sd, sd.toString()
+  config.hasFlowPath(source, sink) and
+  not exists(
+    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/released/0.0.11.md

@@ -0,0 +1,20 @@
+## 0.0.11
+
+### Breaking Changes
+
+* 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.
+
+### Deprecated Predicates and Classes
+
+* The predicates and classes in the `CodeDuplication` library have been deprecated.
+
+### New Queries
+
+* 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.
+* 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.
+
+### Minor Analysis Improvements
+
+* 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'.
+* 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/codeql-pack.release.yml

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

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,14 +2,14 @@
 
 import cpp
 
-private newtype TDuplicationOrSimilarity = MKDuplicationOrSimilarity()
+deprecated private newtype TDuplicationOrSimilarity = MKDuplicationOrSimilarity()
 
 /**
  * DEPRECATED: This class is no longer used.
  *
  * A token block used for detection of duplicate and similar code.
  */
-class Copy extends TDuplicationOrSimilarity {
+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) { none() }
 
@@ -63,7 +63,7 @@ class Copy extends TDuplicationOrSimilarity {
  *
  * A block of duplicated code.
  */
-class DuplicateBlock extends Copy {
+deprecated class DuplicateBlock extends Copy {
   override string toString() {
     result = "Duplicate code: " + this.sourceLines() + " duplicated lines."
   }
@@ -74,21 +74,29 @@ class DuplicateBlock extends Copy {
  *
  * A block of similar code.
  */
-class SimilarBlock extends Copy {
+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
@@ -96,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
@@ -105,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 |
@@ -125,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
@@ -159,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 =
@@ -175,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 |
@@ -191,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
@@ -211,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 =
@@ -236,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
@@ -247,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
@@ -258,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 |
@@ -277,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
     (
@@ -305,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.10
+version: 0.0.11
 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;
+}

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

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

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

@@ -0,0 +1,18 @@
+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, ...);
+char *fgets(char *str, int num, FILE *stream);
+int fclose(FILE *stream);
+
+int main(int argc, char *argv[])
+{
+  FILE *fp;
+  char buf[128];
+  fp = fopen("myFile.txt","r+"); // BAD [NOT DETECTED]
+  fgets(buf,128,fp); 
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+  return 0;
+}

cpp/ql/test/library-tests/ir/ir/ir.cpp

@@ -1471,31 +1471,43 @@ void array_structured_binding() {
     // explicit reference version
     {
         auto& unnamed_local_variable = xs;
-        auto& x0 = xs[0];
-        auto& x1 = xs[1];
+        auto& x0 = unnamed_local_variable[0];
+        auto& x1 = unnamed_local_variable[1];
         x1 = 3;
         int &rx1 = x1;
         int x = x1;
     }
 }
 
+struct StructuredBindingDataMemberMemberStruct {
+    int x = 5;
+};
+
 struct StructuredBindingDataMemberStruct {
+    typedef int ArrayType[2];
+    typedef int &RefType;
     int i = 1;
     double d = 2.0;
     unsigned int b : 3;
     int& r = i;
+    int* p = &i;
+    ArrayType xs = {1, 2};
+    RefType r_alt = i;
+    StructuredBindingDataMemberMemberStruct m;
 };
 
 void data_member_structured_binding() {
     StructuredBindingDataMemberStruct s;
     // structured binding use
     {
-        auto [i, d, b, r] = s;
+        auto [i, d, b, r, p, xs, r_alt, m] = s;
         d = 4.0;
         double& rd = d;
         int v = i;
         r = 5;
+        *p = 6;
         int& rr = r;
+        int* pr = &r;
         int w = r;
     }
     // explicit reference version
@@ -1505,58 +1517,67 @@ void data_member_structured_binding() {
         auto& d = unnamed_local_variable.d;
         // no equivalent for b
         auto& r = unnamed_local_variable.r;
+        auto& p = unnamed_local_variable.p;
         d = 4.0;
         double& rd = d;
         int v = i;
         r = 5;
+        *p = 6;
         int& rr = r;
+        int* pr = &r;
         int w = r;
     }
 }
-struct StructuredBindingTuple;
 
 namespace std {
     template<typename T>
     struct tuple_size;
-    template<>
-    struct tuple_size<StructuredBindingTuple> {
-        static const unsigned int value = 3;
-    };
-
     template<int, typename T>
     struct tuple_element;
-    template<>
-    struct tuple_element<0, StructuredBindingTuple> {
-        using type = int;
-    };
-    template<>
-    struct tuple_element<1, StructuredBindingTuple> {
-        using type = double;
-    };
-    template<>
-    struct tuple_element<2, StructuredBindingTuple> {
-        using type = int&;
-    };
 }
 
-struct StructuredBindingTuple {
+struct StructuredBindingTupleRefGet {
     int i = 1;
     double d = 2.2;
     int& r = i;
 
     template<int i>
-    typename std::tuple_element<i, StructuredBindingTuple>::type& get();
+    typename std::tuple_element<i, StructuredBindingTupleRefGet>::type& get();
 };
 
 template<>
-std::tuple_element<0, StructuredBindingTuple>::type& StructuredBindingTuple::get<0>() { return i; }
-template<>
-std::tuple_element<1, StructuredBindingTuple>::type& StructuredBindingTuple::get<1>() { return d; }
-template<>
-std::tuple_element<2, StructuredBindingTuple>::type& StructuredBindingTuple::get<2>() { return r; }
+struct std::tuple_size<StructuredBindingTupleRefGet> {
+    static const unsigned int value = 3;
+};
 
-void tuple_structured_binding() {
-    StructuredBindingTuple t;
+template<>
+struct std::tuple_element<0, StructuredBindingTupleRefGet> {
+    using type = int;
+};
+template<>
+struct std::tuple_element<1, StructuredBindingTupleRefGet> {
+    using type = double;
+};
+template<>
+struct std::tuple_element<2, StructuredBindingTupleRefGet> {
+    using type = int&;
+};
+
+template<>
+std::tuple_element<0, StructuredBindingTupleRefGet>::type& StructuredBindingTupleRefGet::get<0>() {
+    return i;
+}
+template<>
+std::tuple_element<1, StructuredBindingTupleRefGet>::type& StructuredBindingTupleRefGet::get<1>() {
+    return d;
+}
+template<>
+std::tuple_element<2, StructuredBindingTupleRefGet>::type& StructuredBindingTupleRefGet::get<2>() {
+    return r;
+}
+
+void tuple_structured_binding_ref_get() {
+    StructuredBindingTupleRefGet t;
     // structured binding use
     {
         auto [i, d, r] = t;
@@ -1582,4 +1603,70 @@ void tuple_structured_binding() {
     }
 }
 
+struct StructuredBindingTupleNoRefGet {
+    int i = 1;
+    int& r = i;
+
+    template<int i>
+    typename std::tuple_element<i, StructuredBindingTupleNoRefGet>::type get();
+};
+
+template<>
+struct std::tuple_size<StructuredBindingTupleNoRefGet> {
+    static const unsigned int value = 3;
+};
+
+template<>
+struct std::tuple_element<0, StructuredBindingTupleNoRefGet> {
+    using type = int;
+};
+template<>
+struct std::tuple_element<1, StructuredBindingTupleNoRefGet> {
+    using type = int&;
+};
+template<>
+struct std::tuple_element<2, StructuredBindingTupleNoRefGet> {
+    using type = int&&;
+};
+
+template<>
+std::tuple_element<0, StructuredBindingTupleNoRefGet>::type StructuredBindingTupleNoRefGet::get<0>() {
+    return i;
+}
+template<>
+std::tuple_element<1, StructuredBindingTupleNoRefGet>::type StructuredBindingTupleNoRefGet::get<1>() {
+    return r;
+}
+template<>
+std::tuple_element<2, StructuredBindingTupleNoRefGet>::type StructuredBindingTupleNoRefGet::get<2>() {
+    return 5;
+}
+
+void tuple_structured_binding_no_ref_get() {
+    StructuredBindingTupleNoRefGet t;
+    //structured binding use
+    {
+        auto&& [i, r, rv] = t;
+        i = 4;
+        int& ri = i;
+        int v = i;
+        r = 5;
+        int& rr = r;
+        int w = r;
+    }
+    // explicit reference version
+    {
+        auto&& unnamed_local_variable = t;
+        auto&& i = unnamed_local_variable.get<0>();
+        auto& r = unnamed_local_variable.get<1>();
+        auto&& rv = unnamed_local_variable.get<2>();
+        i = 4;
+        int& ri = i;
+        int v = i;
+        r = 5;
+        int& rr = r;
+        int w = r;
+    }
+}
+
 // semmle-extractor-options: -std=c++17 --clang

cpp/ql/test/library-tests/ir/ir/raw_consistency.expected

@@ -17,6 +17,7 @@ containsLoopOfForwardEdges
 lostReachability
 backEdgeCountMismatch
 useNotDominatedByDefinition
+| ir.cpp:1486:8:1486:8 | Unary | Operand 'Unary' is not dominated by its definition in function '$@'. | ir.cpp:1486:8:1486:8 | void StructuredBindingDataMemberStruct::StructuredBindingDataMemberStruct() | void StructuredBindingDataMemberStruct::StructuredBindingDataMemberStruct() |
 switchInstructionWithoutDefaultEdge
 notMarkedAsConflated
 wronglyMarkedAsConflated

cpp/ql/test/query-tests/Critical/UnsafeUseOfThis/UnsafeUseOfThis.expected

@@ -1,61 +1,103 @@
 edges
-| test.cpp:7:3:7:3 | InitializeParameter: B | test.cpp:8:12:8:15 | Load: this |
-| test.cpp:8:12:8:15 | Load: this | test.cpp:34:16:34:16 | InitializeParameter: x |
-| test.cpp:11:8:11:8 | InitializeParameter: b | test.cpp:12:5:12:5 | Load: b |
-| test.cpp:12:5:12:5 | CopyValue: (reference dereference) | test.cpp:12:5:12:5 | ConvertToNonVirtualBase: (A)... |
-| test.cpp:12:5:12:5 | Load: b | test.cpp:12:5:12:5 | CopyValue: (reference dereference) |
-| test.cpp:15:3:15:4 | InitializeParameter: ~B | test.cpp:16:5:16:5 | Load: this |
-| test.cpp:16:5:16:5 | Load: this | file://:0:0:0:0 | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:21:3:21:3 | InitializeParameter: C | test.cpp:21:13:21:13 | ConvertToNonVirtualBase: call to B |
-| test.cpp:21:3:21:3 | InitializeParameter: C | test.cpp:22:12:22:15 | Load: this |
-| test.cpp:21:3:21:3 | InitializeParameter: C | test.cpp:25:7:25:10 | Load: this |
-| test.cpp:21:13:21:13 | ConvertToNonVirtualBase: call to B | test.cpp:7:3:7:3 | InitializeParameter: B |
-| test.cpp:22:12:22:15 | ConvertToNonVirtualBase: (B *)... | test.cpp:34:16:34:16 | InitializeParameter: x |
-| test.cpp:22:12:22:15 | Load: this | test.cpp:22:12:22:15 | ConvertToNonVirtualBase: (B *)... |
-| test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (B *)... | test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:25:7:25:10 | Load: this | test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (B *)... |
-| test.cpp:31:3:31:3 | InitializeParameter: D | test.cpp:31:12:31:15 | Load: this |
-| test.cpp:31:11:31:15 | ConvertToNonVirtualBase: (B)... | test.cpp:31:11:31:15 | CopyValue: (reference to) |
-| test.cpp:31:11:31:15 | CopyValue: (reference to) | test.cpp:11:8:11:8 | InitializeParameter: b |
-| test.cpp:31:11:31:15 | CopyValue: * ... | test.cpp:31:11:31:15 | ConvertToNonVirtualBase: (B)... |
-| test.cpp:31:12:31:15 | Load: this | test.cpp:31:11:31:15 | CopyValue: * ... |
-| test.cpp:34:16:34:16 | InitializeParameter: x | test.cpp:35:3:35:3 | Load: x |
-| test.cpp:35:3:35:3 | Load: x | test.cpp:35:3:35:3 | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:47:3:47:3 | InitializeParameter: F | test.cpp:48:10:48:13 | Load: this |
-| test.cpp:48:10:48:13 | ConvertToNonVirtualBase: (E *)... | test.cpp:48:6:48:13 | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:48:10:48:13 | Load: this | test.cpp:48:10:48:13 | ConvertToNonVirtualBase: (E *)... |
+| test.cpp:7:3:7:3 | this | test.cpp:8:12:8:15 | Load |
+| test.cpp:8:12:8:15 | Load | test.cpp:8:12:8:15 | this |
+| test.cpp:8:12:8:15 | this | test.cpp:34:16:34:16 | x |
+| test.cpp:11:8:11:8 | b | test.cpp:12:5:12:5 | Load |
+| test.cpp:12:5:12:5 | (reference dereference) | test.cpp:12:5:12:5 | Unary |
+| test.cpp:12:5:12:5 | Load | test.cpp:12:5:12:5 | b |
+| test.cpp:12:5:12:5 | Unary | test.cpp:12:5:12:5 | (A)... |
+| test.cpp:12:5:12:5 | Unary | test.cpp:12:5:12:5 | (reference dereference) |
+| test.cpp:12:5:12:5 | b | test.cpp:12:5:12:5 | Unary |
+| test.cpp:15:3:15:4 | this | test.cpp:16:5:16:5 | Load |
+| test.cpp:16:5:16:5 | Load | test.cpp:16:5:16:5 | this |
+| test.cpp:16:5:16:5 | Unary | file://:0:0:0:0 | (A *)... |
+| test.cpp:16:5:16:5 | this | test.cpp:16:5:16:5 | Unary |
+| test.cpp:21:3:21:3 | Unary | test.cpp:21:13:21:13 | ConvertToNonVirtualBase |
+| test.cpp:21:3:21:3 | this | test.cpp:21:3:21:3 | Unary |
+| test.cpp:21:3:21:3 | this | test.cpp:22:12:22:15 | Load |
+| test.cpp:21:3:21:3 | this | test.cpp:25:7:25:10 | Load |
+| test.cpp:21:13:21:13 | ConvertToNonVirtualBase | test.cpp:7:3:7:3 | this |
+| test.cpp:22:12:22:15 | (B *)... | test.cpp:34:16:34:16 | x |
+| test.cpp:22:12:22:15 | Load | test.cpp:22:12:22:15 | this |
+| test.cpp:22:12:22:15 | Unary | test.cpp:22:12:22:15 | (B *)... |
+| test.cpp:22:12:22:15 | this | test.cpp:22:12:22:15 | Unary |
+| test.cpp:25:7:25:10 | (B *)... | test.cpp:25:7:25:10 | Unary |
+| test.cpp:25:7:25:10 | Load | test.cpp:25:7:25:10 | this |
+| test.cpp:25:7:25:10 | Unary | test.cpp:25:7:25:10 | (A *)... |
+| test.cpp:25:7:25:10 | Unary | test.cpp:25:7:25:10 | (B *)... |
+| test.cpp:25:7:25:10 | this | test.cpp:25:7:25:10 | Unary |
+| test.cpp:31:3:31:3 | this | test.cpp:31:12:31:15 | Load |
+| test.cpp:31:11:31:15 | (B)... | test.cpp:31:11:31:15 | Unary |
+| test.cpp:31:11:31:15 | (reference to) | test.cpp:11:8:11:8 | b |
+| test.cpp:31:11:31:15 | * ... | test.cpp:31:11:31:15 | Unary |
+| test.cpp:31:11:31:15 | Unary | test.cpp:31:11:31:15 | (B)... |
+| test.cpp:31:11:31:15 | Unary | test.cpp:31:11:31:15 | (reference to) |
+| test.cpp:31:12:31:15 | Load | test.cpp:31:12:31:15 | this |
+| test.cpp:31:12:31:15 | Unary | test.cpp:31:11:31:15 | * ... |
+| test.cpp:31:12:31:15 | this | test.cpp:31:12:31:15 | Unary |
+| test.cpp:34:16:34:16 | x | test.cpp:35:3:35:3 | Load |
+| test.cpp:35:3:35:3 | Load | test.cpp:35:3:35:3 | x |
+| test.cpp:35:3:35:3 | Unary | test.cpp:35:3:35:3 | (A *)... |
+| test.cpp:35:3:35:3 | x | test.cpp:35:3:35:3 | Unary |
+| test.cpp:47:3:47:3 | this | test.cpp:48:10:48:13 | Load |
+| test.cpp:48:10:48:13 | (E *)... | test.cpp:48:10:48:13 | Unary |
+| test.cpp:48:10:48:13 | Load | test.cpp:48:10:48:13 | this |
+| test.cpp:48:10:48:13 | Unary | test.cpp:48:6:48:13 | (A *)... |
+| test.cpp:48:10:48:13 | Unary | test.cpp:48:10:48:13 | (E *)... |
+| test.cpp:48:10:48:13 | this | test.cpp:48:10:48:13 | Unary |
 nodes
-| file://:0:0:0:0 | ConvertToNonVirtualBase: (A *)... | semmle.label | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:7:3:7:3 | InitializeParameter: B | semmle.label | InitializeParameter: B |
-| test.cpp:8:12:8:15 | Load: this | semmle.label | Load: this |
-| test.cpp:11:8:11:8 | InitializeParameter: b | semmle.label | InitializeParameter: b |
-| test.cpp:12:5:12:5 | ConvertToNonVirtualBase: (A)... | semmle.label | ConvertToNonVirtualBase: (A)... |
-| test.cpp:12:5:12:5 | CopyValue: (reference dereference) | semmle.label | CopyValue: (reference dereference) |
-| test.cpp:12:5:12:5 | Load: b | semmle.label | Load: b |
-| test.cpp:15:3:15:4 | InitializeParameter: ~B | semmle.label | InitializeParameter: ~B |
-| test.cpp:16:5:16:5 | Load: this | semmle.label | Load: this |
-| test.cpp:21:3:21:3 | InitializeParameter: C | semmle.label | InitializeParameter: C |
-| test.cpp:21:13:21:13 | ConvertToNonVirtualBase: call to B | semmle.label | ConvertToNonVirtualBase: call to B |
-| test.cpp:22:12:22:15 | ConvertToNonVirtualBase: (B *)... | semmle.label | ConvertToNonVirtualBase: (B *)... |
-| test.cpp:22:12:22:15 | Load: this | semmle.label | Load: this |
-| test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (A *)... | semmle.label | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (B *)... | semmle.label | ConvertToNonVirtualBase: (B *)... |
-| test.cpp:25:7:25:10 | Load: this | semmle.label | Load: this |
-| test.cpp:31:3:31:3 | InitializeParameter: D | semmle.label | InitializeParameter: D |
-| test.cpp:31:11:31:15 | ConvertToNonVirtualBase: (B)... | semmle.label | ConvertToNonVirtualBase: (B)... |
-| test.cpp:31:11:31:15 | CopyValue: (reference to) | semmle.label | CopyValue: (reference to) |
-| test.cpp:31:11:31:15 | CopyValue: * ... | semmle.label | CopyValue: * ... |
-| test.cpp:31:12:31:15 | Load: this | semmle.label | Load: this |
-| test.cpp:34:16:34:16 | InitializeParameter: x | semmle.label | InitializeParameter: x |
-| test.cpp:35:3:35:3 | ConvertToNonVirtualBase: (A *)... | semmle.label | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:35:3:35:3 | Load: x | semmle.label | Load: x |
-| test.cpp:47:3:47:3 | InitializeParameter: F | semmle.label | InitializeParameter: F |
-| test.cpp:48:6:48:13 | ConvertToNonVirtualBase: (A *)... | semmle.label | ConvertToNonVirtualBase: (A *)... |
-| test.cpp:48:10:48:13 | ConvertToNonVirtualBase: (E *)... | semmle.label | ConvertToNonVirtualBase: (E *)... |
-| test.cpp:48:10:48:13 | Load: this | semmle.label | Load: this |
+| file://:0:0:0:0 | (A *)... | semmle.label | (A *)... |
+| test.cpp:7:3:7:3 | this | semmle.label | this |
+| test.cpp:8:12:8:15 | Load | semmle.label | Load |
+| test.cpp:8:12:8:15 | this | semmle.label | this |
+| test.cpp:11:8:11:8 | b | semmle.label | b |
+| test.cpp:12:5:12:5 | (A)... | semmle.label | (A)... |
+| test.cpp:12:5:12:5 | (reference dereference) | semmle.label | (reference dereference) |
+| test.cpp:12:5:12:5 | Load | semmle.label | Load |
+| test.cpp:12:5:12:5 | Unary | semmle.label | Unary |
+| test.cpp:12:5:12:5 | Unary | semmle.label | Unary |
+| test.cpp:12:5:12:5 | b | semmle.label | b |
+| test.cpp:15:3:15:4 | this | semmle.label | this |
+| test.cpp:16:5:16:5 | Load | semmle.label | Load |
+| test.cpp:16:5:16:5 | Unary | semmle.label | Unary |
+| test.cpp:16:5:16:5 | this | semmle.label | this |
+| test.cpp:21:3:21:3 | Unary | semmle.label | Unary |
+| test.cpp:21:3:21:3 | this | semmle.label | this |
+| test.cpp:21:13:21:13 | ConvertToNonVirtualBase | semmle.label | ConvertToNonVirtualBase |
+| test.cpp:22:12:22:15 | (B *)... | semmle.label | (B *)... |
+| test.cpp:22:12:22:15 | Load | semmle.label | Load |
+| test.cpp:22:12:22:15 | Unary | semmle.label | Unary |
+| test.cpp:22:12:22:15 | this | semmle.label | this |
+| test.cpp:25:7:25:10 | (A *)... | semmle.label | (A *)... |
+| test.cpp:25:7:25:10 | (B *)... | semmle.label | (B *)... |
+| test.cpp:25:7:25:10 | Load | semmle.label | Load |
+| test.cpp:25:7:25:10 | Unary | semmle.label | Unary |
+| test.cpp:25:7:25:10 | Unary | semmle.label | Unary |
+| test.cpp:25:7:25:10 | this | semmle.label | this |
+| test.cpp:31:3:31:3 | this | semmle.label | this |
+| test.cpp:31:11:31:15 | (B)... | semmle.label | (B)... |
+| test.cpp:31:11:31:15 | (reference to) | semmle.label | (reference to) |
+| test.cpp:31:11:31:15 | * ... | semmle.label | * ... |
+| test.cpp:31:11:31:15 | Unary | semmle.label | Unary |
+| test.cpp:31:11:31:15 | Unary | semmle.label | Unary |
+| test.cpp:31:12:31:15 | Load | semmle.label | Load |
+| test.cpp:31:12:31:15 | Unary | semmle.label | Unary |
+| test.cpp:31:12:31:15 | this | semmle.label | this |
+| test.cpp:34:16:34:16 | x | semmle.label | x |
+| test.cpp:35:3:35:3 | (A *)... | semmle.label | (A *)... |
+| test.cpp:35:3:35:3 | Load | semmle.label | Load |
+| test.cpp:35:3:35:3 | Unary | semmle.label | Unary |
+| test.cpp:35:3:35:3 | x | semmle.label | x |
+| test.cpp:47:3:47:3 | this | semmle.label | this |
+| test.cpp:48:6:48:13 | (A *)... | semmle.label | (A *)... |
+| test.cpp:48:10:48:13 | (E *)... | semmle.label | (E *)... |
+| test.cpp:48:10:48:13 | Load | semmle.label | Load |
+| test.cpp:48:10:48:13 | Unary | semmle.label | Unary |
+| test.cpp:48:10:48:13 | Unary | semmle.label | Unary |
+| test.cpp:48:10:48:13 | this | semmle.label | this |
 #select
-| test.cpp:12:7:12:7 | call to f | test.cpp:31:3:31:3 | InitializeParameter: D | test.cpp:12:5:12:5 | ConvertToNonVirtualBase: (A)... | Call to pure virtual function during construction |
-| test.cpp:16:5:16:5 | call to f | test.cpp:15:3:15:4 | InitializeParameter: ~B | file://:0:0:0:0 | ConvertToNonVirtualBase: (A *)... | Call to pure virtual function during destruction |
-| test.cpp:25:13:25:13 | call to f | test.cpp:21:3:21:3 | InitializeParameter: C | test.cpp:25:7:25:10 | ConvertToNonVirtualBase: (A *)... | Call to pure virtual function during construction |
-| test.cpp:35:6:35:6 | call to f | test.cpp:7:3:7:3 | InitializeParameter: B | test.cpp:35:3:35:3 | ConvertToNonVirtualBase: (A *)... | Call to pure virtual function during construction |
-| test.cpp:35:6:35:6 | call to f | test.cpp:21:3:21:3 | InitializeParameter: C | test.cpp:35:3:35:3 | ConvertToNonVirtualBase: (A *)... | Call to pure virtual function during construction |
+| test.cpp:12:7:12:7 | call to f | test.cpp:31:3:31:3 | this | test.cpp:12:5:12:5 | (A)... | Call to pure virtual function during construction |
+| test.cpp:16:5:16:5 | call to f | test.cpp:15:3:15:4 | this | file://:0:0:0:0 | (A *)... | Call to pure virtual function during destruction |
+| test.cpp:25:13:25:13 | call to f | test.cpp:21:3:21:3 | this | test.cpp:25:7:25:10 | (A *)... | Call to pure virtual function during construction |
+| test.cpp:35:6:35:6 | call to f | test.cpp:7:3:7:3 | this | test.cpp:35:3:35:3 | (A *)... | Call to pure virtual function during construction |
+| test.cpp:35:6:35:6 | call to f | test.cpp:21:3:21:3 | this | test.cpp:35:3:35:3 | (A *)... | Call to pure virtual function during construction |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/ReturnStackAllocatedMemory.expected

@@ -100,6 +100,12 @@ edges
 | test.cpp:190:10:190:13 | Unary | test.cpp:190:10:190:13 | (reference dereference) |
 | test.cpp:190:10:190:13 | Unary | test.cpp:190:10:190:13 | (reference to) |
 | test.cpp:190:10:190:13 | pRef | test.cpp:190:10:190:13 | Unary |
+| test.cpp:225:14:225:15 | px | test.cpp:226:10:226:11 | Load |
+| test.cpp:226:10:226:11 | Load | test.cpp:226:10:226:11 | px |
+| test.cpp:226:10:226:11 | px | test.cpp:226:10:226:11 | StoreValue |
+| test.cpp:231:16:231:17 | & ... | test.cpp:225:14:225:15 | px |
+| test.cpp:231:17:231:17 | Unary | test.cpp:231:16:231:17 | & ... |
+| test.cpp:231:17:231:17 | x | test.cpp:231:17:231:17 | Unary |
 nodes
 | test.cpp:17:9:17:11 | & ... | semmle.label | & ... |
 | test.cpp:17:9:17:11 | StoreValue | semmle.label | StoreValue |
@@ -215,6 +221,13 @@ nodes
 | test.cpp:190:10:190:13 | Unary | semmle.label | Unary |
 | test.cpp:190:10:190:13 | Unary | semmle.label | Unary |
 | test.cpp:190:10:190:13 | pRef | semmle.label | pRef |
+| test.cpp:225:14:225:15 | px | semmle.label | px |
+| test.cpp:226:10:226:11 | Load | semmle.label | Load |
+| test.cpp:226:10:226:11 | StoreValue | semmle.label | StoreValue |
+| test.cpp:226:10:226:11 | px | semmle.label | px |
+| test.cpp:231:16:231:17 | & ... | semmle.label | & ... |
+| test.cpp:231:17:231:17 | Unary | semmle.label | Unary |
+| test.cpp:231:17:231:17 | x | semmle.label | x |
 #select
 | test.cpp:17:9:17:11 | StoreValue | test.cpp:17:10:17:11 | mc | test.cpp:17:9:17:11 | StoreValue | May return stack-allocated memory from $@. | test.cpp:17:10:17:11 | mc | mc |
 | test.cpp:25:9:25:11 | StoreValue | test.cpp:23:18:23:19 | mc | test.cpp:25:9:25:11 | StoreValue | May return stack-allocated memory from $@. | test.cpp:23:18:23:19 | mc | mc |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ReturnStackAllocatedMemory/test.cpp

@@ -220,4 +220,13 @@ auto make_read_port()
 void* get_sp() {
 	int p;
 	return (void*)&p; // GOOD: The function name makes it sound like the programmer intended to get the value of the stack pointer.
+}
+
+int* id(int* px) {
+  return px; // GOOD
+}
+
+void f() {
+  int x;
+  int* px = id(&x); // GOOD
 }

cpp/ql/test/query-tests/Likely Bugs/Memory Management/UsingExpiredStackAddress/UsingExpiredStackAddress.expected

@@ -0,0 +1,92 @@
+edges
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:14:3:14:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:19:3:19:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:28:3:28:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:28:3:28:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:37:3:37:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:37:3:37:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:210:3:210:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:210:3:210:9 | Call: call to escape1 |
+| test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:210:3:210:9 | Call: call to escape1 |
+| test.cpp:14:3:14:9 | Call: call to escape1 | test.cpp:15:16:15:16 | Load: p |
+| test.cpp:23:5:23:11 | EnterFunction: deref_p | test.cpp:24:16:24:16 | Load: p |
+| test.cpp:28:3:28:9 | Call: call to escape1 | test.cpp:23:5:23:11 | EnterFunction: deref_p |
+| test.cpp:28:3:28:9 | Call: call to escape1 | test.cpp:24:16:24:16 | Load: p |
+| test.cpp:37:3:37:9 | Call: call to escape1 | test.cpp:32:5:32:11 | EnterFunction: deref_i |
+| test.cpp:52:3:52:13 | Store: ... = ... | test.cpp:57:3:57:27 | Call: call to store_address_of_argument |
+| test.cpp:57:3:57:27 | Call: call to store_address_of_argument | test.cpp:58:16:58:16 | Load: p |
+| test.cpp:68:3:68:13 | Store: ... = ... | test.cpp:72:3:72:39 | Call: call to address_escapes_through_pointer_arith |
+| test.cpp:68:3:68:13 | Store: ... = ... | test.cpp:78:3:78:39 | Call: call to address_escapes_through_pointer_arith |
+| test.cpp:68:3:68:13 | Store: ... = ... | test.cpp:86:5:86:41 | Call: call to address_escapes_through_pointer_arith |
+| test.cpp:72:3:72:39 | Call: call to address_escapes_through_pointer_arith | test.cpp:73:16:73:16 | Load: p |
+| test.cpp:78:3:78:39 | Call: call to address_escapes_through_pointer_arith | test.cpp:80:16:80:16 | Load: p |
+| test.cpp:93:3:93:15 | Store: ... = ... | test.cpp:97:3:97:23 | Call: call to field_address_escapes |
+| test.cpp:97:3:97:23 | Call: call to field_address_escapes | test.cpp:98:15:98:15 | Load: p |
+| test.cpp:106:3:106:14 | Store: ... = ... | test.cpp:110:3:110:26 | Call: call to escape_through_reference |
+| test.cpp:110:3:110:26 | Call: call to escape_through_reference | test.cpp:111:16:111:16 | Load: p |
+| test.cpp:136:3:136:12 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:137:3:137:16 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:139:3:139:12 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:140:3:140:16 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:141:3:141:15 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:142:3:142:19 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:144:3:144:12 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:145:3:145:16 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:146:3:146:15 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:147:3:147:19 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:148:3:148:18 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:149:3:149:22 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:151:3:151:15 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:152:3:152:19 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:153:3:153:18 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:154:3:154:22 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:155:3:155:21 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:156:3:156:25 | Store: ... = ... | test.cpp:160:3:160:23 | Call: call to escape_through_arrays |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:161:16:161:17 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:162:16:162:17 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:164:16:164:17 | Load: p2 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:165:16:165:17 | Load: p2 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:166:17:166:18 | Load: p2 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:167:16:167:17 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:168:17:168:18 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:170:16:170:17 | Load: p3 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:171:17:171:18 | Load: p3 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:172:18:172:19 | Load: p2 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:173:18:173:19 | Load: p2 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:174:18:174:19 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:175:16:175:17 | Load: p1 |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:177:14:177:21 | Load: access to array |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:178:14:178:21 | Load: access to array |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:179:14:179:21 | Load: access to array |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:180:14:180:19 | Load: * ... |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:181:13:181:20 | Load: access to array |
+| test.cpp:160:3:160:23 | Call: call to escape_through_arrays | test.cpp:182:14:182:19 | Load: * ... |
+| test.cpp:201:5:201:17 | EnterFunction: maybe_deref_p | test.cpp:201:5:201:17 | VariableAddress: maybe_deref_p |
+| test.cpp:210:3:210:9 | Call: call to escape1 | test.cpp:201:5:201:17 | EnterFunction: maybe_deref_p |
+| test.cpp:210:3:210:9 | Call: call to escape1 | test.cpp:201:5:201:17 | VariableAddress: maybe_deref_p |
+#select
+| test.cpp:15:16:15:16 | Load: p | test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:15:16:15:16 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:9:7:9:7 | x | x | test.cpp:10:3:10:13 | Store: ... = ... | here |
+| test.cpp:24:16:24:16 | Load: p | test.cpp:10:3:10:13 | Store: ... = ... | test.cpp:24:16:24:16 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:9:7:9:7 | x | x | test.cpp:10:3:10:13 | Store: ... = ... | here |
+| test.cpp:58:16:58:16 | Load: p | test.cpp:52:3:52:13 | Store: ... = ... | test.cpp:58:16:58:16 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:51:36:51:36 | y | y | test.cpp:52:3:52:13 | Store: ... = ... | here |
+| test.cpp:73:16:73:16 | Load: p | test.cpp:68:3:68:13 | Store: ... = ... | test.cpp:73:16:73:16 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:62:7:62:7 | x | x | test.cpp:68:3:68:13 | Store: ... = ... | here |
+| test.cpp:98:15:98:15 | Load: p | test.cpp:93:3:93:15 | Store: ... = ... | test.cpp:98:15:98:15 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:92:8:92:8 | s | s | test.cpp:93:3:93:15 | Store: ... = ... | here |
+| test.cpp:111:16:111:16 | Load: p | test.cpp:106:3:106:14 | Store: ... = ... | test.cpp:111:16:111:16 | Load: p | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:102:7:102:7 | x | x | test.cpp:106:3:106:14 | Store: ... = ... | here |
+| test.cpp:161:16:161:17 | Load: p1 | test.cpp:136:3:136:12 | Store: ... = ... | test.cpp:161:16:161:17 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:132:7:132:8 | b1 | b1 | test.cpp:136:3:136:12 | Store: ... = ... | here |
+| test.cpp:162:16:162:17 | Load: p1 | test.cpp:137:3:137:16 | Store: ... = ... | test.cpp:162:16:162:17 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:132:7:132:8 | b1 | b1 | test.cpp:137:3:137:16 | Store: ... = ... | here |
+| test.cpp:164:16:164:17 | Load: p2 | test.cpp:139:3:139:12 | Store: ... = ... | test.cpp:164:16:164:17 | Load: p2 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:139:3:139:12 | Store: ... = ... | here |
+| test.cpp:165:16:165:17 | Load: p2 | test.cpp:139:3:139:12 | Store: ... = ... | test.cpp:165:16:165:17 | Load: p2 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:139:3:139:12 | Store: ... = ... | here |
+| test.cpp:166:17:166:18 | Load: p2 | test.cpp:140:3:140:16 | Store: ... = ... | test.cpp:166:17:166:18 | Load: p2 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:140:3:140:16 | Store: ... = ... | here |
+| test.cpp:167:16:167:17 | Load: p1 | test.cpp:141:3:141:15 | Store: ... = ... | test.cpp:167:16:167:17 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:141:3:141:15 | Store: ... = ... | here |
+| test.cpp:168:17:168:18 | Load: p1 | test.cpp:142:3:142:19 | Store: ... = ... | test.cpp:168:17:168:18 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:142:3:142:19 | Store: ... = ... | here |
+| test.cpp:170:16:170:17 | Load: p3 | test.cpp:144:3:144:12 | Store: ... = ... | test.cpp:170:16:170:17 | Load: p3 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:144:3:144:12 | Store: ... = ... | here |
+| test.cpp:171:17:171:18 | Load: p3 | test.cpp:145:3:145:16 | Store: ... = ... | test.cpp:171:17:171:18 | Load: p3 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:145:3:145:16 | Store: ... = ... | here |
+| test.cpp:172:18:172:19 | Load: p2 | test.cpp:146:3:146:15 | Store: ... = ... | test.cpp:172:18:172:19 | Load: p2 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:146:3:146:15 | Store: ... = ... | here |
+| test.cpp:173:18:173:19 | Load: p2 | test.cpp:147:3:147:19 | Store: ... = ... | test.cpp:173:18:173:19 | Load: p2 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:147:3:147:19 | Store: ... = ... | here |
+| test.cpp:174:18:174:19 | Load: p1 | test.cpp:142:3:142:19 | Store: ... = ... | test.cpp:174:18:174:19 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:142:3:142:19 | Store: ... = ... | here |
+| test.cpp:175:16:175:17 | Load: p1 | test.cpp:148:3:148:18 | Store: ... = ... | test.cpp:175:16:175:17 | Load: p1 | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:148:3:148:18 | Store: ... = ... | here |
+| test.cpp:177:14:177:21 | Load: access to array | test.cpp:151:3:151:15 | Store: ... = ... | test.cpp:177:14:177:21 | Load: access to array | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:132:7:132:8 | b1 | b1 | test.cpp:151:3:151:15 | Store: ... = ... | here |
+| test.cpp:178:14:178:21 | Load: access to array | test.cpp:152:3:152:19 | Store: ... = ... | test.cpp:178:14:178:21 | Load: access to array | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:132:7:132:8 | b1 | b1 | test.cpp:152:3:152:19 | Store: ... = ... | here |
+| test.cpp:179:14:179:21 | Load: access to array | test.cpp:153:3:153:18 | Store: ... = ... | test.cpp:179:14:179:21 | Load: access to array | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:153:3:153:18 | Store: ... = ... | here |
+| test.cpp:180:14:180:19 | Load: * ... | test.cpp:154:3:154:22 | Store: ... = ... | test.cpp:180:14:180:19 | Load: * ... | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:133:7:133:8 | b2 | b2 | test.cpp:154:3:154:22 | Store: ... = ... | here |
+| test.cpp:181:13:181:20 | Load: access to array | test.cpp:155:3:155:21 | Store: ... = ... | test.cpp:181:13:181:20 | Load: access to array | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:155:3:155:21 | Store: ... = ... | here |
+| test.cpp:182:14:182:19 | Load: * ... | test.cpp:156:3:156:25 | Store: ... = ... | test.cpp:182:14:182:19 | Load: * ... | Stack variable $@ escapes $@ and is used after it has expired. | test.cpp:134:7:134:8 | b3 | b3 | test.cpp:156:3:156:25 | Store: ... = ... | here |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/UsingExpiredStackAddress/UsingExpiredStackAddress.qlref

@@ -0,0 +1 @@
+Likely Bugs/Memory Management/UsingExpiredStackAddress.ql

cpp/ql/test/query-tests/Likely Bugs/Memory Management/UsingExpiredStackAddress/test.cpp

@@ -0,0 +1,212 @@
+struct S100 {
+  int i;
+  int* p;
+};
+
+static struct S100 s101;
+
+void escape1() {
+  int x;
+  s101.p = &x;
+}
+
+int simple_field_bad() {
+  escape1();
+  return *s101.p; // BAD
+}
+
+int simple_field_good() {
+  escape1();
+  return s101.i; // GOOD
+}
+
+int deref_p() {
+  return *s101.p; // BAD
+}
+
+int field_indirect_bad() {
+  escape1();
+  return deref_p();
+}
+
+int deref_i() {
+  return s101.i;
+}
+
+int field_indirect_good() {
+  escape1();
+  return deref_i(); // GOOD
+}
+
+void store_argument(int *p) {
+  s101.p = p;
+}
+
+int store_argument_value() {
+  int x;
+  store_argument(&x);
+  return *s101.p; // GOOD
+}
+
+void store_address_of_argument(int y) {
+  s101.p = &y;
+}
+
+int store_argument_address() {
+  int x;
+  store_address_of_argument(x);
+  return *s101.p; // BAD
+}
+
+void address_escapes_through_pointer_arith() {
+  int x[2];
+  int* p0 = x;
+  int* p1 = p0 + 1;
+  int* p2 = p1 - 1;
+  int* p3 = 1 + p2;
+  p3++;
+  s101.p = p3;
+}
+
+int test_pointer_arith_bad() {
+  address_escapes_through_pointer_arith();
+  return *s101.p; // BAD
+}
+
+int test_pointer_arith_good_1() {
+  int x;
+  address_escapes_through_pointer_arith();
+  s101.p = &x;
+  return *s101.p; // GOOD
+}
+
+int test_pointer_arith_good_2(bool b) {
+  int x;
+  if(b) {
+    address_escapes_through_pointer_arith();
+  }
+  return *s101.p; // GOOD (we can't say for sure that this is a local address)
+}
+
+void field_address_escapes() {
+  S100 s;
+  s101.p = &s.i;
+}
+
+int test_field_address_escapes() {
+  field_address_escapes();
+  return s101.p[0]; // BAD
+}
+
+void escape_through_reference() {
+  int x = 0;
+  int& r0 = x;
+  int& r1 = r0;
+  r1++;
+  s101.p = &r1;
+}
+
+int test_escapes_through_reference() {
+  escape_through_reference();
+  return *s101.p; // BAD
+}
+
+struct S300 {
+  int a1[15];
+  int a2[14][15];
+  int a3[13][14][15];
+  int *p1;
+  int (*p2)[15];
+  int (*p3)[14][15];
+  int** pp;
+};
+
+S300 s1;
+S300 s2;
+S300 s3;
+S300 s4;
+S300 s5;
+S300 s6;
+
+void escape_through_arrays() {
+  int b1[15];
+  int b2[14][15];
+  int b3[13][14][15];
+
+  s1.p1 = b1;
+  s2.p1 = &b1[1];
+
+  s1.p2 = b2;
+  s2.p2 = &b2[1];
+  s3.p1 = b2[1];
+  s4.p1 = &b2[1][2];
+
+  s1.p3 = b3;
+  s2.p3 = &b3[1];
+  s3.p2 = b3[1];
+  s4.p2 = &b3[1][2];
+  s5.p1 = b3[1][2];
+  s6.p1 = &b3[1][2][3];
+
+  s1.pp[0] = b1;
+  s2.pp[0] = &b1[1];
+  s3.pp[0] = b2[1];
+  s4.pp[0] = &b2[1][2];
+  s5.pp[0] = b3[1][2];
+  s6.pp[0] = &b3[1][2][3];
+}
+
+void test_escape_through_arrays() {
+  escape_through_arrays();
+  int x1 = *s1.p1; // BAD
+  int x2 = *s2.p1; // BAD
+
+  int* x3 = s1.p2[1]; // BAD
+  int x4 = *s1.p2[1]; // BAD
+  int* x5 = *s2.p2; // BAD
+  int* x6 = s3.p1; // BAD
+  int x7 = *&s4.p1[1]; // BAD
+
+  int x8 = *s1.p3[1][2]; // BAD
+  int x9 = (*s2.p3[0])[0]; // BAD
+  int x10 = **s3.p2; // BAD
+  int x11 = **s4.p2; // BAD
+  int x12 = (*s4.p1); // BAD
+  int x13 = s5.p1[1]; // BAD
+
+  int* x14 = s1.pp[0]; // BAD
+  int x15 = *s2.pp[0]; // BAD
+  int x16 = *s3.pp[0]; // BAD
+  int x17 = **s4.pp; // BAD
+  int x18 = s5.pp[0][0]; // BAD
+  int x19 = (*s6.pp)[0]; // BAD
+}
+
+void not_escape_through_arrays() {
+  int x;
+
+  s1.a1[0] = x;
+  s1.a2[0][1] = s1.a1[0];
+  **s1.a3[0] = 42;
+}
+
+void test_not_escape_through_array() {
+  not_escape_through_arrays();
+  
+  int x20 = s1.a1[0]; // GOOD
+  int x21 = s1.a2[0][1]; // GOOD
+  int* x22 = s1.a3[5][2]; // GOOD
+}
+
+int maybe_deref_p(bool b) {
+  if(b) {
+    return *s101.p; // GOOD
+  } else {
+    return 0;
+  }
+}
+
+int field_indirect_maybe_bad(bool b) {
+  escape1();
+  return maybe_deref_p(b);
+}

cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/UseOfHttp.expected

@@ -7,6 +7,8 @@ edges
 | test.cpp:40:11:40:17 | access to array | test.cpp:11:26:11:28 | url |
 | test.cpp:46:18:46:26 | http:// | test.cpp:49:11:49:16 | buffer |
 | test.cpp:49:11:49:16 | buffer | test.cpp:11:26:11:28 | url |
+| test.cpp:110:21:110:40 | http://example.com | test.cpp:121:11:121:13 | ptr |
+| test.cpp:121:11:121:13 | ptr | test.cpp:11:26:11:28 | url |
 nodes
 | test.cpp:11:26:11:28 | url | semmle.label | url |
 | test.cpp:15:30:15:32 | url | semmle.label | url |
@@ -17,9 +19,12 @@ nodes
 | test.cpp:40:11:40:17 | access to array | semmle.label | access to array |
 | test.cpp:46:18:46:26 | http:// | semmle.label | http:// |
 | test.cpp:49:11:49:16 | buffer | semmle.label | buffer |
+| test.cpp:110:21:110:40 | http://example.com | semmle.label | http://example.com |
+| test.cpp:121:11:121:13 | ptr | semmle.label | ptr |
 subpaths
 #select
 | test.cpp:28:10:28:29 | http://example.com | test.cpp:28:10:28:29 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:35:23:35:42 | http://example.com | test.cpp:35:23:35:42 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:36:26:36:45 | http://example.com | test.cpp:36:26:36:45 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
 | test.cpp:46:18:46:26 | http:// | test.cpp:46:18:46:26 | http:// | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |
+| test.cpp:110:21:110:40 | http://example.com | test.cpp:110:21:110:40 | http://example.com | test.cpp:15:30:15:32 | url | A URL may be constructed with the HTTP protocol. |

cpp/ql/test/query-tests/Security/CWE/CWE-319/UseOfHttp/test.cpp

@@ -58,3 +58,66 @@ void test()
 		openUrl(buffer);
 	}
 }
+
+typedef unsigned long size_t;
+int strncmp(const char *s1, const char *s2, size_t n);
+char* strstr(char* s1, const char* s2);
+
+void test2(const char *url)
+{
+	if (strncmp(url, "http://", 7)) // GOOD (or at least dubious; we are not constructing the URL)
+	{
+		openUrl(url);
+	}
+}
+
+void test3(char *url)
+{
+	char *ptr;
+
+	ptr = strstr(url, "https://"); // GOOD (https)
+	if (!ptr)
+	{
+		ptr = strstr(url, "http://"); // GOOD (we are not constructing the URL)
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}
+
+void test4(char *url)
+{
+	const char *https_string = "https://"; // GOOD (https)
+	const char *http_string = "http://"; // GOOD (we are not constructing the URL)
+	char *ptr;
+
+	ptr = strstr(url, https_string);
+	if (!ptr)
+	{
+		ptr = strstr(url, http_string);
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}
+
+void test5()
+{
+	char *url_string = "http://example.com"; // BAD
+	char *ptr;
+
+	ptr = strstr(url_string, "https://"); // GOOD (https)
+	if (!ptr)
+	{
+		ptr = strstr(url_string, "http://"); // GOOD (we are not constructing the URL here)
+	}
+
+	if (ptr)
+	{
+		openUrl(ptr);
+	}
+}

cpp/ql/test/query-tests/Security/CWE/CWE-326/InsufficientKeySize.expected

@@ -0,0 +1,10 @@
+edges
+nodes
+| test.cpp:34:45:34:48 | 1024 | semmle.label | 1024 |
+| test.cpp:35:49:35:52 | 1024 | semmle.label | 1024 |
+| test.cpp:37:43:37:46 | 1024 | semmle.label | 1024 |
+subpaths
+#select
+| test.cpp:34:5:34:38 | call to EVP_PKEY_CTX_set_dsa_paramgen_bits | test.cpp:34:45:34:48 | 1024 | test.cpp:34:45:34:48 | 1024 | The key size $@ is less than the recommended key size of 2048 bits. | test.cpp:34:45:34:48 | 1024 | 1024 |
+| test.cpp:35:5:35:42 | call to EVP_PKEY_CTX_set_dh_paramgen_prime_len | test.cpp:35:49:35:52 | 1024 | test.cpp:35:49:35:52 | 1024 | The key size $@ is less than the recommended key size of 2048 bits. | test.cpp:35:49:35:52 | 1024 | 1024 |
+| test.cpp:37:5:37:36 | call to EVP_PKEY_CTX_set_rsa_keygen_bits | test.cpp:37:43:37:46 | 1024 | test.cpp:37:43:37:46 | 1024 | The key size $@ is less than the recommended key size of 2048 bits. | test.cpp:37:43:37:46 | 1024 | 1024 |

cpp/ql/test/query-tests/Security/CWE/CWE-326/InsufficientKeySize.qlref

@@ -0,0 +1 @@
+Security/CWE/CWE-326/InsufficientKeySize.ql

cpp/ql/test/query-tests/Security/CWE/CWE-326/test.cpp

@@ -0,0 +1,38 @@
+
+
+typedef int EVP_CIPHER;
+typedef int EVP_MD;
+
+const EVP_CIPHER *EVP_aes_128_ctr();
+const EVP_CIPHER *EVP_aes_192_ctr();
+const EVP_CIPHER *EVP_aes_256_ctr();
+
+const EVP_MD *EVP_sha224();
+const EVP_MD *EVP_sha256();
+const EVP_MD *EVP_sha384();
+const EVP_MD *EVP_sha512();
+
+
+class EVP_PKEY_CTX;
+
+ // int is a curve ID rather than a bit width
+int EVP_PKEY_CTX_set_ec_paramgen_curve_nid(EVP_PKEY_CTX*, int);
+
+int EVP_PKEY_CTX_set_dsa_paramgen_bits(EVP_PKEY_CTX*, int);
+int EVP_PKEY_CTX_set_dh_paramgen_prime_len(EVP_PKEY_CTX*, int);
+
+// RSA sets bits per-key rather than with parameters
+int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX*, int);
+
+void test1(EVP_PKEY_CTX *ctx) {
+    EVP_PKEY_CTX_set_dsa_paramgen_bits(ctx, 2048);
+    EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, 2048);
+    // RSA sets bits per-key rather than with parameters
+    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 2048);
+
+    // low key sizes
+    EVP_PKEY_CTX_set_dsa_paramgen_bits(ctx, 1024);
+    EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, 1024);
+    // RSA sets bits per-key rather than with parameters
+    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, 1024);
+}

cpp/ql/test/query-tests/Security/CWE/CWE-497/SAMATE/ExposedSystemData.expected

@@ -1 +1,4 @@
-| tests.c:70:9:70:15 | call to fprintf | This operation exposes system data from $@. | tests.c:54:13:54:22 | call to LogonUserA | call to LogonUserA |
+edges
+nodes
+subpaths
+#select

cpp/ql/test/query-tests/Security/CWE/CWE-497/SAMATE/OutputWrite.expected

@@ -1,7 +0,0 @@
-| tests.c:29:9:29:14 | call to printf | tests.c:29:16:29:21 | %s\n |
-| tests.c:29:9:29:14 | call to printf | tests.c:29:24:29:27 | line |
-| tests.c:43:13:43:21 | call to printLine | tests.c:43:23:43:38 | fgets() failed |
-| tests.c:62:13:62:21 | call to printLine | tests.c:62:23:62:52 | User logged in successfully. |
-| tests.c:67:13:67:21 | call to printLine | tests.c:67:23:67:40 | Unable to login. |
-| tests.c:70:9:70:15 | call to fprintf | tests.c:70:25:70:67 | User attempted access with password: %s\n |
-| tests.c:70:9:70:15 | call to fprintf | tests.c:70:70:70:77 | password |

cpp/ql/test/query-tests/Security/CWE/CWE-497/SAMATE/OutputWrite.ql

@@ -1,4 +0,0 @@
-import semmle.code.cpp.security.OutputWrite
-
-from OutputWrite ow
-select ow, ow.getASource()

cpp/ql/test/query-tests/Security/CWE/CWE-497/SAMATE/tests.c

@@ -67,6 +67,6 @@ void CWE535_Info_Exposure_Shell_Error__w32_char_01_bad()
             printLine("Unable to login.");
         }
         /* FLAW: Write sensitive data to stderr */
-        fprintf(stderr, "User attempted access with password: %s\n", password);
+        fprintf(stderr, "User attempted access with password: %s\n", password); // [NOT DETECTED]
     }
 }

cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/ExposedSystemData.expected

@@ -1,2 +1,47 @@
-| tests2.cpp:27:12:27:12 | call to operator<< | This operation exposes system data from $@. | tests2.cpp:27:15:27:20 | call to getenv | call to getenv |
-| tests2.cpp:28:25:28:25 | call to operator<< | This operation exposes system data from $@. | tests2.cpp:28:28:28:33 | call to getenv | call to getenv |
+edges
+| tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:26 | (const char *)... |
+| tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:26 | (const char *)... |
+| tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:30 | (const char *)... |
+| tests2.cpp:76:18:76:38 | call to mysql_get_client_info | tests2.cpp:79:14:79:19 | (const char *)... |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info |
+| tests2.cpp:89:42:89:45 | str1 | tests2.cpp:91:14:91:17 | str1 |
+| tests2.cpp:99:8:99:15 | call to getpwuid | tests2.cpp:100:14:100:15 | pw |
+| tests2.cpp:107:3:107:4 | c1 [post update] [ptr] | tests2.cpp:109:14:109:15 | c1 [read] [ptr] |
+| tests2.cpp:107:6:107:8 | ptr [post update] | tests2.cpp:107:3:107:4 | c1 [post update] [ptr] |
+| tests2.cpp:107:12:107:17 | call to getenv | tests2.cpp:107:6:107:8 | ptr [post update] |
+| tests2.cpp:109:14:109:15 | c1 [read] [ptr] | tests2.cpp:109:14:109:19 | (const char *)... |
+nodes
+| tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:63:13:63:26 | (const char *)... | semmle.label | (const char *)... |
+| tests2.cpp:64:13:64:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:64:13:64:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:64:13:64:26 | (const char *)... | semmle.label | (const char *)... |
+| tests2.cpp:65:13:65:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:65:13:65:18 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:65:13:65:30 | (const char *)... | semmle.label | (const char *)... |
+| tests2.cpp:76:18:76:38 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info |
+| tests2.cpp:79:14:79:19 | (const char *)... | semmle.label | (const char *)... |
+| tests2.cpp:89:42:89:45 | str1 | semmle.label | str1 |
+| tests2.cpp:91:14:91:17 | str1 | semmle.label | str1 |
+| tests2.cpp:99:8:99:15 | call to getpwuid | semmle.label | call to getpwuid |
+| tests2.cpp:100:14:100:15 | pw | semmle.label | pw |
+| tests2.cpp:107:3:107:4 | c1 [post update] [ptr] | semmle.label | c1 [post update] [ptr] |
+| tests2.cpp:107:6:107:8 | ptr [post update] | semmle.label | ptr [post update] |
+| tests2.cpp:107:12:107:17 | call to getenv | semmle.label | call to getenv |
+| tests2.cpp:109:14:109:15 | c1 [read] [ptr] | semmle.label | c1 [read] [ptr] |
+| tests2.cpp:109:14:109:19 | (const char *)... | semmle.label | (const char *)... |
+subpaths
+#select
+| tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:63:13:63:18 | call to getenv | call to getenv |
+| tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:64:13:64:18 | call to getenv | call to getenv |
+| tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:65:13:65:18 | call to getenv | call to getenv |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | This operation exposes system data from $@. | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | call to mysql_get_client_info |
+| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | This operation exposes system data from $@. | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | call to mysql_get_client_info |
+| tests2.cpp:79:14:79:19 | (const char *)... | tests2.cpp:76:18:76:38 | call to mysql_get_client_info | tests2.cpp:79:14:79:19 | (const char *)... | This operation exposes system data from $@. | tests2.cpp:76:18:76:38 | call to mysql_get_client_info | call to mysql_get_client_info |
+| tests2.cpp:91:14:91:17 | str1 | tests2.cpp:89:42:89:45 | str1 | tests2.cpp:91:14:91:17 | str1 | This operation exposes system data from $@. | tests2.cpp:89:42:89:45 | str1 | str1 |
+| tests2.cpp:100:14:100:15 | pw | tests2.cpp:99:8:99:15 | call to getpwuid | tests2.cpp:100:14:100:15 | pw | This operation exposes system data from $@. | tests2.cpp:99:8:99:15 | call to getpwuid | call to getpwuid |
+| tests2.cpp:109:14:109:19 | (const char *)... | tests2.cpp:107:12:107:17 | call to getenv | tests2.cpp:109:14:109:19 | (const char *)... | This operation exposes system data from $@. | tests2.cpp:107:12:107:17 | call to getenv | call to getenv |

cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/OutputWrite.expected

@@ -1,5 +0,0 @@
-| tests2.cpp:27:12:27:12 | call to operator<< | tests2.cpp:27:15:27:20 | call to getenv |
-| tests2.cpp:28:12:28:12 | call to operator<< | tests2.cpp:28:15:28:23 | PATH =  |
-| tests2.cpp:28:25:28:25 | call to operator<< | tests2.cpp:28:28:28:33 | call to getenv |
-| tests2.cpp:28:43:28:43 | call to operator<< | tests2.cpp:28:46:28:48 | . |
-| tests2.cpp:29:12:29:12 | call to operator<< | tests2.cpp:29:15:29:28 | PATHPATHPATH |

cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/OutputWrite.ql

@@ -1,4 +0,0 @@
-import semmle.code.cpp.security.OutputWrite
-
-from OutputWrite ow
-select ow, ow.getASource()

cpp/ql/test/query-tests/Security/CWE/CWE-497/semmle/tests/tests2.cpp

@@ -3,6 +3,7 @@
 // library functions etc
 
 char *getenv(const char *name);
+char *strcpy(char *s1, const char *s2);
 
 namespace std
 {
@@ -20,11 +21,92 @@ namespace std
 	extern ostream cout;
 }
 
+int socket(int p1, int p2, int p3);
+void send(int sock, const char *buffer, int p3, int p4);
+
+const char *mysql_get_client_info();
+void mysql_real_connect(int p1, int p2, int p3, const char *password, int p5, int p6, int p7, int p8);
+
+struct container
+{
+	char *ptr;
+};
+
+struct passwd
+{
+	// ...
+
+	char *pw_passwd;
+
+	// ...
+};
+
+passwd *getpwuid(int uid);
+
+int val();
+
 // test cases
 
+const char *global1 = mysql_get_client_info();
+const char *global2 = "abc";
+
 void test1()
 {
-	std::cout << getenv("HOME"); // BAD: outputs HOME environment variable
-	std::cout << "PATH = " << getenv("PATH") << "."; // BAD: outputs PATH environment variable
+	int sock = socket(val(), val(), val());
+
+	// tests for a strict implementation of CWE-497
+	std::cout << getenv("HOME"); // BAD: outputs HOME environment variable [NOT DETECTED]
+	std::cout << "PATH = " << getenv("PATH") << "."; // BAD: outputs PATH environment variable [NOT DETECTED]
 	std::cout << "PATHPATHPATH"; // GOOD: not system data
+
+	// tests for a more pragmatic implementation of CWE-497
+	send(sock, getenv("HOME"), val(), val()); // BAD
+	send(sock, getenv("PATH"), val(), val()); // BAD
+	send(sock, getenv("USERNAME"), val(), val()); // BAD
+	send(sock, getenv("HARMLESS"), val(), val()); // GOOD: harmless information
+	send(sock, "HOME", val(), val()); // GOOD: not system data
+	send(sock, "PATH", val(), val()); // GOOD: not system data
+	send(sock, "USERNAME", val(), val()); // GOOD: not system data
+	send(sock, "HARMLESS", val(), val()); // GOOD: not system data
+
+	// tests for `mysql_get_client_info`, including via a global
+	{
+		char buffer[256];
+
+		strcpy(buffer, mysql_get_client_info());
+
+		send(sock, mysql_get_client_info(), val(), val()); // BAD
+		send(sock, buffer, val(), val()); // BAD
+		send(sock, global1, val(), val()); // BAD [NOT DETECTED]
+		send(sock, global2, val(), val()); // GOOD: not system data
+	}
+
+	// tests for `mysql_real_connect`
+	{
+		const char *str1 = "123456";
+		const char *str2 = "abcdef";
+
+		mysql_real_connect(sock, val(), val(), str1, val(), val(), val(), val());
+
+		send(sock, str1, val(), val()); // BAD
+		send(sock, str2, val(), val()); // GOOD: not system data
+	}
+
+	// tests for `getpwuid`
+	{
+		passwd *pw;
+
+		pw = getpwuid(val());
+		send(sock, pw->pw_passwd, val(), val()); // BAD
+	}
+
+	// tests for containers
+	{
+		container c1, c2;
+
+		c1.ptr = getenv("MY_SECRET_TOKEN");
+		c2.ptr = "";
+		send(sock, c1.ptr, val(), val()); // BAD
+		send(sock, c2.ptr, val(), val()); // GOOD: not system data
+	}
 }

csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs

@@ -51,7 +51,7 @@ namespace Semmle.Autobuild.CSharp.Tests
 
         int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env, out IList<string> stdOut)
         {
-            var pattern = cmd + " " + args;
+            var pattern = string.IsNullOrEmpty(args) ? cmd : cmd + " " + args;
             RunProcessIn.Add(pattern);
 
             if (!RunProcessOut.TryGetValue(pattern, out var str))
@@ -62,7 +62,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             RunProcessWorkingDirectory.TryGetValue(pattern, out var wd);
 
             if (wd != workingDirectory)
-                throw new ArgumentException("Missing RunProcessWorkingDirectory " + pattern);
+                throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}");
 
             if (!RunProcess.TryGetValue(pattern, out var ret))
                 throw new ArgumentException("Missing RunProcess " + pattern);
@@ -72,12 +72,12 @@ namespace Semmle.Autobuild.CSharp.Tests
 
         int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env)
         {
-            var pattern = cmd + " " + args;
+            var pattern = string.IsNullOrEmpty(args) ? cmd : cmd + " " + args;
             RunProcessIn.Add(pattern);
             RunProcessWorkingDirectory.TryGetValue(pattern, out var wd);
 
             if (wd != workingDirectory)
-                throw new ArgumentException("Missing RunProcessWorkingDirectory " + pattern);
+                throw new ArgumentException($"Unexpected RunProcessWorkingDirectory, got {wd ?? "null"} expected {workingDirectory ?? "null"} in {pattern}");
 
             if (!RunProcess.TryGetValue(pattern, out var ret))
                 throw new ArgumentException("Missing RunProcess " + pattern);
@@ -255,7 +255,7 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestAnd1()
         {
-            var cmd = BuildScript.Create("abc", "def ghi", false, null, null) & BuildScript.Create("odasa", null, false, null, null);
+            var cmd = BuildScript.Create("abc", "def ghi", false, null, null) & BuildScript.Create("codeql", null, false, null, null);
 
             actions.RunProcess["abc def ghi"] = 1;
             cmd.Run(actions, StartCallback, EndCallback);
@@ -269,14 +269,14 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestAnd2()
         {
-            var cmd = BuildScript.Create("odasa", null, false, null, null) & BuildScript.Create("abc", "def ghi", false, null, null);
+            var cmd = BuildScript.Create("codeql", null, false, null, null) & BuildScript.Create("abc", "def ghi", false, null, null);
 
             actions.RunProcess["abc def ghi"] = 1;
-            actions.RunProcess["odasa "] = 0;
+            actions.RunProcess["codeql"] = 0;
             cmd.Run(actions, StartCallback, EndCallback);
 
-            Assert.Equal("odasa ", actions.RunProcessIn[0]);
-            Assert.Equal("odasa ", startCallbackIn[0]);
+            Assert.Equal("codeql", actions.RunProcessIn[0]);
+            Assert.Equal("codeql", startCallbackIn[0]);
             Assert.Equal("", endCallbackIn[0]);
             Assert.Equal(0, endCallbackReturn[0]);
 
@@ -289,14 +289,14 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestOr1()
         {
-            var cmd = BuildScript.Create("odasa", null, false, null, null) | BuildScript.Create("abc", "def ghi", false, null, null);
+            var cmd = BuildScript.Create("codeql", null, false, null, null) | BuildScript.Create("abc", "def ghi", false, null, null);
 
             actions.RunProcess["abc def ghi"] = 1;
-            actions.RunProcess["odasa "] = 0;
+            actions.RunProcess["codeql"] = 0;
             cmd.Run(actions, StartCallback, EndCallback);
 
-            Assert.Equal("odasa ", actions.RunProcessIn[0]);
-            Assert.Equal("odasa ", startCallbackIn[0]);
+            Assert.Equal("codeql", actions.RunProcessIn[0]);
+            Assert.Equal("codeql", startCallbackIn[0]);
             Assert.Equal("", endCallbackIn[0]);
             Assert.Equal(0, endCallbackReturn[0]);
             Assert.Equal(1, endCallbackReturn.Count);
@@ -305,10 +305,10 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestOr2()
         {
-            var cmd = BuildScript.Create("abc", "def ghi", false, null, null) | BuildScript.Create("odasa", null, false, null, null);
+            var cmd = BuildScript.Create("abc", "def ghi", false, null, null) | BuildScript.Create("codeql", null, false, null, null);
 
             actions.RunProcess["abc def ghi"] = 1;
-            actions.RunProcess["odasa "] = 0;
+            actions.RunProcess["codeql"] = 0;
             cmd.Run(actions, StartCallback, EndCallback);
 
             Assert.Equal("abc def ghi", actions.RunProcessIn[0]);
@@ -316,8 +316,8 @@ namespace Semmle.Autobuild.CSharp.Tests
             Assert.Equal("", endCallbackIn[0]);
             Assert.Equal(1, endCallbackReturn[0]);
 
-            Assert.Equal("odasa ", actions.RunProcessIn[1]);
-            Assert.Equal("odasa ", startCallbackIn[1]);
+            Assert.Equal("codeql", actions.RunProcessIn[1]);
+            Assert.Equal("codeql", startCallbackIn[1]);
             Assert.Equal("", endCallbackIn[1]);
             Assert.Equal(0, endCallbackReturn[1]);
         }
@@ -385,9 +385,6 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable[$"CODEQL_EXTRACTOR_{codeqlUpperLanguage}_ROOT"] = $@"C:\codeql\{codeqlUpperLanguage.ToLowerInvariant()}";
             actions.GetEnvironmentVariable["CODEQL_JAVA_HOME"] = @"C:\codeql\tools\java";
             actions.GetEnvironmentVariable["CODEQL_PLATFORM"] = isWindows ? "win64" : "linux64";
-            actions.GetEnvironmentVariable["SEMMLE_DIST"] = @"C:\odasa";
-            actions.GetEnvironmentVariable["SEMMLE_JAVA_HOME"] = @"C:\odasa\tools\java";
-            actions.GetEnvironmentVariable["SEMMLE_PLATFORM_TOOLS"] = @"C:\odasa\tools";
             actions.GetEnvironmentVariable["LGTM_INDEX_VSTOOLS_VERSION"] = vsToolsVersion;
             actions.GetEnvironmentVariable["LGTM_INDEX_MSBUILD_ARGUMENTS"] = msBuildArguments;
             actions.GetEnvironmentVariable["LGTM_INDEX_MSBUILD_PLATFORM"] = msBuildPlatform;
@@ -399,6 +396,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable["LGTM_INDEX_SOLUTION"] = solution;
             actions.GetEnvironmentVariable["LGTM_INDEX_IGNORE_ERRORS"] = ignoreErrors;
             actions.GetEnvironmentVariable["LGTM_INDEX_BUILDLESS"] = buildless;
+            actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_OPTION_BUILDLESS"] = buildless;
             actions.GetEnvironmentVariable["LGTM_INDEX_ALL_SOLUTIONS"] = allSolutions;
             actions.GetEnvironmentVariable["LGTM_INDEX_NUGET_RESTORE"] = nugetRestore;
             actions.GetEnvironmentVariable["ProgramFiles(x86)"] = isWindows ? @"C:\Program Files (x86)" : null;
@@ -415,7 +413,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess["cmd.exe /C dotnet --info"] = 0;
             actions.RunProcess[@"cmd.exe /C dotnet clean C:\Project\test.csproj"] = 0;
             actions.RunProcess[@"cmd.exe /C dotnet restore C:\Project\test.csproj"] = 0;
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project\test.csproj"] = 0;
+            actions.RunProcess[@"cmd.exe /C dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project\test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project\test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -442,7 +440,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess["dotnet --info"] = 0;
             actions.RunProcess[@"dotnet clean C:\Project/test.csproj"] = 0;
             actions.RunProcess[@"dotnet restore C:\Project/test.csproj"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
+            actions.RunProcess[@"dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project/test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -600,7 +598,7 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestLinuxBuildCommand()
         {
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto ""./build.sh --skip-tests"""] = 0;
+            actions.RunProcess["./build.sh --skip-tests"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
@@ -621,8 +619,8 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
             actions.RunProcess[@"/bin/chmod u+x C:\Project/build/build.sh"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/build/build.sh"] = 0;
-            actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto C:\Project/build/build.sh"] = @"C:\Project/build";
+            actions.RunProcess[@"C:\Project/build/build.sh"] = 0;
+            actions.RunProcessWorkingDirectory[@"C:\Project/build/build.sh"] = @"C:\Project/build";
             actions.FileExists["csharp.log"] = true;
 
             var autobuilder = CreateAutoBuilder(false);
@@ -638,8 +636,8 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
 
             actions.RunProcess[@"/bin/chmod u+x C:\Project/build.sh"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/build.sh"] = 0;
-            actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto C:\Project/build.sh"] = @"C:\Project";
+            actions.RunProcess[@"C:\Project/build.sh"] = 0;
+            actions.RunProcessWorkingDirectory[@"C:\Project/build.sh"] = @"C:\Project";
             actions.FileExists["csharp.log"] = false;
 
             var autobuilder = CreateAutoBuilder(false);
@@ -655,8 +653,8 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
 
             actions.RunProcess[@"/bin/chmod u+x C:\Project/build.sh"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/build.sh"] = 5;
-            actions.RunProcessWorkingDirectory[@"C:\odasa/tools/odasa index --auto C:\Project/build.sh"] = @"C:\Project";
+            actions.RunProcess[@"C:\Project/build.sh"] = 5;
+            actions.RunProcessWorkingDirectory[@"C:\Project/build.sh"] = @"C:\Project";
             actions.FileExists["csharp.log"] = true;
 
             var autobuilder = CreateAutoBuilder(false);
@@ -670,8 +668,8 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.EnumerateDirectories[@"C:\Project"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\build.bat"] = 0;
-            actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\build.bat"] = @"C:\Project";
+            actions.RunProcess[@"cmd.exe /C C:\Project\build.bat"] = 0;
+            actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\Project\build.bat"] = @"C:\Project";
             actions.FileExists["csharp.log"] = true;
 
             var autobuilder = CreateAutoBuilder(true);
@@ -685,10 +683,10 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.EnumerateDirectories[@"C:\Project"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SOURCE_ARCHIVE_DIR"] = "";
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\build.bat"] = 1;
-            actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\build.bat"] = @"C:\Project";
+            actions.RunProcess[@"cmd.exe /C C:\Project\build.bat"] = 1;
+            actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\Project\build.bat"] = @"C:\Project";
             actions.RunProcess[@"cmd.exe /C C:\codeql\tools\java\bin\java -jar C:\codeql\csharp\tools\extractor-asp.jar ."] = 0;
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0;
+            actions.RunProcess[@"cmd.exe /C C:\codeql\tools\codeql index --xml --extensions config"] = 0;
             actions.FileExists["csharp.log"] = true;
 
             var autobuilder = CreateAutoBuilder(true, ignoreErrors: "true");
@@ -698,9 +696,9 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestWindowsCmdIgnoreErrors()
         {
-            actions.RunProcess["cmd.exe /C C:\\odasa\\tools\\odasa index --auto ^\"build.cmd --skip-tests^\""] = 3;
+            actions.RunProcess["cmd.exe /C ^\"build.cmd --skip-tests^\""] = 3;
             actions.RunProcess[@"cmd.exe /C C:\codeql\tools\java\bin\java -jar C:\codeql\csharp\tools\extractor-asp.jar ."] = 0;
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0;
+            actions.RunProcess[@"cmd.exe /C C:\codeql\tools\codeql index --xml --extensions config"] = 0;
             actions.FileExists["csharp.log"] = true;
             SkipVsWhere();
 
@@ -717,9 +715,9 @@ namespace Semmle.Autobuild.CSharp.Tests
         public void TestWindowCSharpMsBuild()
         {
             actions.RunProcess[@"cmd.exe /C C:\Project\.nuget\nuget.exe restore C:\Project\test1.sln -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.RunProcess[@"cmd.exe /C C:\Project\.nuget\nuget.exe restore C:\Project\test2.sln -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false;
@@ -748,9 +746,9 @@ namespace Semmle.Autobuild.CSharp.Tests
         public void TestWindowCSharpMsBuildMultipleSolutions()
         {
             actions.RunProcess[@"cmd.exe /C nuget restore C:\Project\test1.csproj -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test1.csproj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test1.csproj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.RunProcess[@"cmd.exe /C nuget restore C:\Project\test2.csproj -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.csproj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test2.csproj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project\test1.csproj"] = true;
             actions.FileExists[@"C:\Project\test2.csproj"] = true;
@@ -793,7 +791,7 @@ namespace Semmle.Autobuild.CSharp.Tests
         public void TestWindowCSharpMsBuildFailed()
         {
             actions.RunProcess[@"cmd.exe /C nuget restore C:\Project\test1.sln -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 1;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 1;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false;
@@ -819,8 +817,8 @@ namespace Semmle.Autobuild.CSharp.Tests
         [Fact]
         public void TestSkipNugetMsBuild()
         {
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false;
             actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false;
@@ -864,7 +862,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess["dotnet --info"] = 0;
             actions.RunProcess[@"dotnet clean C:\Project/test.csproj"] = 0;
             actions.RunProcess[@"dotnet restore C:\Project/test.csproj"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false --no-restore C:\Project/test.csproj"] = 0;
+            actions.RunProcess[@"dotnet build --no-incremental /p:UseSharedCompilation=false --no-restore C:\Project/test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project/test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -896,7 +894,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess[@"C:\Project/.dotnet/dotnet --info"] = 0;
             actions.RunProcess[@"C:\Project/.dotnet/dotnet clean C:\Project/test.csproj"] = 0;
             actions.RunProcess[@"C:\Project/.dotnet/dotnet restore C:\Project/test.csproj"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/.dotnet/dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
+            actions.RunProcess[@"C:\Project/.dotnet/dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists["test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -931,7 +929,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess[@"C:\Project/.dotnet/dotnet --info"] = 0;
             actions.RunProcess[@"C:\Project/.dotnet/dotnet clean C:\Project/test.csproj"] = 0;
             actions.RunProcess[@"C:\Project/.dotnet/dotnet restore C:\Project/test.csproj"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto C:\Project/.dotnet/dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
+            actions.RunProcess[@"C:\Project/.dotnet/dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project/test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists["test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -962,7 +960,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet --info"] = 0;
             actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet clean C:\Project\test.csproj"] = 0;
             actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet restore C:\Project\test.csproj"] = 0;
-            actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project\test.csproj"] = 0;
+            actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false C:\Project\test.csproj"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project\test.csproj"] = true;
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_TRAP_DIR"] = "";
@@ -1010,7 +1008,7 @@ namespace Semmle.Autobuild.CSharp.Tests
         {
             actions.RunProcess[@"cmd.exe /C nuget restore C:\Project\dirs.proj -DisableParallelProcessing"] = 1;
             actions.RunProcess[@"cmd.exe /C C:\Project\.nuget\nuget.exe restore C:\Project\dirs.proj -DisableParallelProcessing"] = 0;
-            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\dirs.proj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
+            actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && set Platform=&& type NUL && msbuild C:\\Project\\dirs.proj /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project\a\test.csproj"] = true;
             actions.FileExists[@"C:\Project\dirs.proj"] = true;
@@ -1054,7 +1052,7 @@ namespace Semmle.Autobuild.CSharp.Tests
         {
             actions.RunProcess[@"nuget restore C:\Project/dirs.proj -DisableParallelProcessing"] = 1;
             actions.RunProcess[@"mono C:\Project/.nuget/nuget.exe restore C:\Project/dirs.proj -DisableParallelProcessing"] = 0;
-            actions.RunProcess[@"C:\odasa/tools/odasa index --auto msbuild C:\Project/dirs.proj /p:UseSharedCompilation=false /t:rebuild /p:MvcBuildViews=true"] = 0;
+            actions.RunProcess[@"msbuild C:\Project/dirs.proj /p:UseSharedCompilation=false /t:rebuild /p:MvcBuildViews=true"] = 0;
             actions.FileExists["csharp.log"] = true;
             actions.FileExists[@"C:\Project/a/test.csproj"] = true;
             actions.FileExists[@"C:\Project/dirs.proj"] = true;

csharp/autobuilder/Semmle.Autobuild.CSharp/DotNetRule.cs

@@ -240,7 +240,7 @@ namespace Semmle.Autobuild.CSharp
         private static BuildScript GetBuildScript(Autobuilder builder, string? dotNetPath, IDictionary<string, string>? environment, string projOrSln)
         {
             var build = new CommandBuilder(builder.Actions, null, environment);
-            var script = builder.MaybeIndex(build, DotNetCommand(builder.Actions, dotNetPath)).
+            var script = build.RunCommand(DotNetCommand(builder.Actions, dotNetPath)).
                 Argument("build").
                 Argument("--no-incremental");
 

csharp/autobuilder/Semmle.Autobuild.CSharp/StandaloneBuildRule.cs

@@ -17,10 +17,6 @@ namespace Semmle.Autobuild.CSharp
                 {
                     standalone = builder.Actions.PathCombine(builder.CodeQLExtractorLangRoot, "tools", builder.CodeQlPlatform, "Semmle.Extraction.CSharp.Standalone");
                 }
-                else if (builder.SemmlePlatformTools is not null)
-                {
-                    standalone = builder.Actions.PathCombine(builder.SemmlePlatformTools, "csharp", "Semmle.Extraction.CSharp.Standalone");
-                }
                 else
                 {
                     return BuildScript.Failure;

csharp/autobuilder/Semmle.Autobuild.Shared/AutobuildOptions.cs

@@ -10,7 +10,8 @@ namespace Semmle.Autobuild.Shared
     /// </summary>
     public class AutobuildOptions
     {
-        private const string prefix = "LGTM_INDEX_";
+        private const string lgtmPrefix = "LGTM_INDEX_";
+        private const string extractorOptionPrefix = "CODEQL_EXTRACTOR_CSHARP_OPTION_";
 
         public int SearchDepth { get; } = 3;
         public string RootDirectory { get; }
@@ -36,20 +37,21 @@ namespace Semmle.Autobuild.Shared
         public AutobuildOptions(IBuildActions actions, Language language)
         {
             RootDirectory = actions.GetCurrentDirectory();
-            VsToolsVersion = actions.GetEnvironmentVariable(prefix + "VSTOOLS_VERSION");
-            MsBuildArguments = actions.GetEnvironmentVariable(prefix + "MSBUILD_ARGUMENTS")?.AsStringWithExpandedEnvVars(actions);
-            MsBuildPlatform = actions.GetEnvironmentVariable(prefix + "MSBUILD_PLATFORM");
-            MsBuildConfiguration = actions.GetEnvironmentVariable(prefix + "MSBUILD_CONFIGURATION");
-            MsBuildTarget = actions.GetEnvironmentVariable(prefix + "MSBUILD_TARGET");
-            DotNetArguments = actions.GetEnvironmentVariable(prefix + "DOTNET_ARGUMENTS")?.AsStringWithExpandedEnvVars(actions);
-            DotNetVersion = actions.GetEnvironmentVariable(prefix + "DOTNET_VERSION");
-            BuildCommand = actions.GetEnvironmentVariable(prefix + "BUILD_COMMAND");
-            Solution = actions.GetEnvironmentVariable(prefix + "SOLUTION").AsListWithExpandedEnvVars(actions, Array.Empty<string>());
+            VsToolsVersion = actions.GetEnvironmentVariable(lgtmPrefix + "VSTOOLS_VERSION");
+            MsBuildArguments = actions.GetEnvironmentVariable(lgtmPrefix + "MSBUILD_ARGUMENTS")?.AsStringWithExpandedEnvVars(actions);
+            MsBuildPlatform = actions.GetEnvironmentVariable(lgtmPrefix + "MSBUILD_PLATFORM");
+            MsBuildConfiguration = actions.GetEnvironmentVariable(lgtmPrefix + "MSBUILD_CONFIGURATION");
+            MsBuildTarget = actions.GetEnvironmentVariable(lgtmPrefix + "MSBUILD_TARGET");
+            DotNetArguments = actions.GetEnvironmentVariable(lgtmPrefix + "DOTNET_ARGUMENTS")?.AsStringWithExpandedEnvVars(actions);
+            DotNetVersion = actions.GetEnvironmentVariable(lgtmPrefix + "DOTNET_VERSION");
+            BuildCommand = actions.GetEnvironmentVariable(lgtmPrefix + "BUILD_COMMAND");
+            Solution = actions.GetEnvironmentVariable(lgtmPrefix + "SOLUTION").AsListWithExpandedEnvVars(actions, Array.Empty<string>());
 
-            IgnoreErrors = actions.GetEnvironmentVariable(prefix + "IGNORE_ERRORS").AsBool("ignore_errors", false);
-            Buildless = actions.GetEnvironmentVariable(prefix + "BUILDLESS").AsBool("buildless", false);
-            AllSolutions = actions.GetEnvironmentVariable(prefix + "ALL_SOLUTIONS").AsBool("all_solutions", false);
-            NugetRestore = actions.GetEnvironmentVariable(prefix + "NUGET_RESTORE").AsBool("nuget_restore", true);
+            IgnoreErrors = actions.GetEnvironmentVariable(lgtmPrefix + "IGNORE_ERRORS").AsBool("ignore_errors", false);
+            Buildless = actions.GetEnvironmentVariable(lgtmPrefix + "BUILDLESS").AsBool("buildless", false) ||
+                actions.GetEnvironmentVariable(extractorOptionPrefix + "BUILDLESS").AsBool("buildless", false);
+            AllSolutions = actions.GetEnvironmentVariable(lgtmPrefix + "ALL_SOLUTIONS").AsBool("all_solutions", false);
+            NugetRestore = actions.GetEnvironmentVariable(lgtmPrefix + "NUGET_RESTORE").AsBool("nuget_restore", true);
 
             Language = language;
         }
@@ -62,21 +64,12 @@ namespace Semmle.Autobuild.Shared
             if (value is null)
                 return defaultValue;
 
-            switch (value.ToLower())
+            return value.ToLower() switch
             {
-                case "on":
-                case "yes":
-                case "true":
-                case "enabled":
-                    return true;
-                case "off":
-                case "no":
-                case "false":
-                case "disabled":
-                    return false;
-                default:
-                    throw new ArgumentOutOfRangeException(param, value, "The Boolean value is invalid.");
-            }
+                "on" or "yes" or "true" or "enabled" => true,
+                "off" or "no" or "false" or "disabled" => false,
+                _ => throw new ArgumentOutOfRangeException(param, value, "The Boolean value is invalid."),
+            };
         }
 
         public static string[] AsListWithExpandedEnvVars(this string? value, IBuildActions actions, string[] defaultValue)

csharp/autobuilder/Semmle.Autobuild.Shared/Autobuilder.cs

@@ -190,19 +190,15 @@ namespace Semmle.Autobuild.Shared
             });
 
             CodeQLExtractorLangRoot = Actions.GetEnvironmentVariable($"CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_ROOT");
-            SemmlePlatformTools = Actions.GetEnvironmentVariable("SEMMLE_PLATFORM_TOOLS");
-
             CodeQlPlatform = Actions.GetEnvironmentVariable("CODEQL_PLATFORM");
 
             TrapDir =
                 Actions.GetEnvironmentVariable($"CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_TRAP_DIR") ??
-                Actions.GetEnvironmentVariable("TRAP_FOLDER") ??
-                throw new InvalidEnvironmentException($"The environment variable CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_TRAP_DIR or TRAP_FOLDER has not been set.");
+                throw new InvalidEnvironmentException($"The environment variable CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_TRAP_DIR has not been set.");
 
             SourceArchiveDir =
                 Actions.GetEnvironmentVariable($"CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_SOURCE_ARCHIVE_DIR") ??
-                Actions.GetEnvironmentVariable("SOURCE_ARCHIVE") ??
-                throw new InvalidEnvironmentException($"The environment variable CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_SOURCE_ARCHIVE_DIR or SOURCE_ARCHIVE has not been set.");
+                throw new InvalidEnvironmentException($"The environment variable CODEQL_EXTRACTOR_{this.Options.Language.UpperCaseName}_SOURCE_ARCHIVE_DIR has not been set.");
         }
 
         protected string TrapDir { get; }
@@ -264,34 +260,9 @@ namespace Semmle.Autobuild.Shared
         /// </summary>
         public string? CodeQLExtractorLangRoot { get; }
 
-        /// <summary>
-        /// Value of SEMMLE_PLATFORM_TOOLS environment variable.
-        /// </summary>
-        public string? SemmlePlatformTools { get; }
-
         /// <summary>
         /// Value of CODEQL_PLATFORM environment variable.
         /// </summary>
         public string? CodeQlPlatform { get; }
-
-        /// <summary>
-        /// The absolute path of the odasa executable.
-        /// null if we are running in CodeQL.
-        /// </summary>
-        public string? Odasa
-        {
-            get
-            {
-                var semmleDist = Actions.GetEnvironmentVariable("SEMMLE_DIST");
-                return semmleDist is null ? null : Actions.PathCombine(semmleDist, "tools", "odasa");
-            }
-        }
-
-        /// <summary>
-        /// Construct a command that executed the given <paramref name="cmd"/> wrapped in
-        /// an <code>odasa --index</code>, unless indexing has been disabled, in which case
-        /// <paramref name="cmd"/> is run directly.
-        /// </summary>
-        public CommandBuilder MaybeIndex(CommandBuilder builder, string cmd) => Odasa is null ? builder.RunCommand(cmd) : builder.IndexCommand(Odasa, cmd);
     }
 }

csharp/autobuilder/Semmle.Autobuild.Shared/BuildCommandAutoRule.cs

@@ -58,7 +58,7 @@ namespace Semmle.Autobuild.Shared
                 if (vsTools is not null)
                     command.CallBatFile(vsTools.Path);
 
-                builder.MaybeIndex(command, scriptPath);
+                command.RunCommand(scriptPath);
                 return command.Script;
             });
         }

csharp/autobuilder/Semmle.Autobuild.Shared/BuildCommandRule.cs

@@ -26,7 +26,7 @@
                     var vsTools = MsBuildRule.GetVcVarsBatFile(builder);
                     if (vsTools is not null)
                         command.CallBatFile(vsTools.Path);
-                    builder.MaybeIndex(command, builder.Options.BuildCommand);
+                    command.RunCommand(builder.Options.BuildCommand);
 
                     return command.Script;
                 });

csharp/autobuilder/Semmle.Autobuild.Shared/BuildScript.cs

@@ -70,7 +70,7 @@ namespace Semmle.Autobuild.Shared
                 this.environment = environment;
             }
 
-            public override string ToString() => exe + " " + arguments;
+            public override string ToString() => arguments.Length > 0 ? exe + " " + arguments : exe;
 
             public override int Run(IBuildActions actions, Action<string, bool> startCallback, Action<int, string, bool> exitCallBack)
             {