Merge pull request #10716 from github/release-prep/2.11.1

Release preparation for version 2.11.1

.github/workflows/go-tests.yml

@@ -43,7 +43,7 @@ jobs:
           env QHELP_OUT_DIR=qhelp-out make qhelp-to-markdown
 
       - name: Upload qhelp markdown
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v3
         with:
           name: qhelp-markdown
           path: go/qhelp-out/**/*.md

.github/workflows/qhelp-pr-preview.yml

@@ -27,7 +27,7 @@ on:
       - main
       - "rc/*"
     paths:
-      - "ruby/**/*.qhelp"
+      - "**/*.qhelp"
 
 jobs:
   qhelp:

README.md

@@ -4,8 +4,7 @@ This open source repository contains the standard CodeQL libraries and queries t
 
 ## How do I learn CodeQL and run queries?
 
-There is [extensive documentation](https://codeql.github.com/docs/) on getting started with writing CodeQL.
-You can use the [CodeQL for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) extension or the [interactive query console](https://lgtm.com/help/lgtm/using-query-console) on LGTM.com (Semmle Legacy product) to try out your queries on any open source project that's currently being analyzed.
+There is [extensive documentation](https://codeql.github.com/docs/) on getting started with writing CodeQL using the [CodeQL extension for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) and the [CodeQL CLI](https://codeql.github.com/docs/codeql-cli/).
 
 ## Contributing
 

config/identical-files.json

@@ -33,8 +33,9 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForLibraries.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForRegExp.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForHttpClientLibraries.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForPathname.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl.qll"
   ],
   "DataFlow Java/C++/C#/Python Common": [
@@ -69,7 +70,7 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking3/TaintTrackingImpl.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/tainttracking4/TaintTrackingImpl.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/tainttracking1/TaintTrackingImpl.qll",
-    "ruby/ql/lib/codeql/ruby/dataflow/internal/tainttrackingforlibraries/TaintTrackingImpl.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/tainttrackingforregexp/TaintTrackingImpl.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/tainttracking1/TaintTrackingImpl.qll"
   ],
   "DataFlow Java/C++/C#/Python Consistency checks": [

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,22 @@
+## 0.4.1
+
+No user-facing changes.
+
+## 0.4.0
+
+### Deprecated APIs
+
+* Some classes/modules with upper-case acronyms in their name have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.
+
+### New Features
+
+* Added subclasses of `BuiltInOperations` for `__is_same`, `__is_function`, `__is_layout_compatible`, `__is_pointer_interconvertible_base_of`, `__is_array`, `__array_rank`, `__array_extent`, `__is_arithmetic`, `__is_complete_type`, `__is_compound`, `__is_const`, `__is_floating_point`, `__is_fundamental`, `__is_integral`, `__is_lvalue_reference`, `__is_member_function_pointer`, `__is_member_object_pointer`, `__is_member_pointer`, `__is_object`, `__is_pointer`, `__is_reference`, `__is_rvalue_reference`, `__is_scalar`, `__is_signed`, `__is_unsigned`, `__is_void`, and `__is_volatile`.
+
+### Bug Fixes
+
+* Fixed an issue in the taint tracking analysis where implicit reads were not allowed by default in sinks or additional taint steps that used flow states.
+
 ## 0.3.5
 
 ## 0.3.4

cpp/ql/lib/change-notes/2022-09-08-implicit-read-flowstates.md

@@ -1,4 +0,0 @@
----
-category: fix
----
-* Fixed an issue in the taint tracking analysis where implicit reads were not allowed by default in sinks or additional taint steps that used flow states.

cpp/ql/lib/change-notes/2022-09-12-uppercase.md

@@ -1,5 +0,0 @@
----
-category: deprecated
----
-* Some classes/modules with upper-case acronyms in their name have been renamed to follow our style-guide. 
-  The old name still exists as a deprecated alias.

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

@@ -1,4 +1,14 @@
----
-category: feature
----
+## 0.4.0
+
+### Deprecated APIs
+
+* Some classes/modules with upper-case acronyms in their name have been renamed to follow our style-guide. 
+  The old name still exists as a deprecated alias.
+
+### New Features
+
 * Added subclasses of `BuiltInOperations` for `__is_same`, `__is_function`, `__is_layout_compatible`, `__is_pointer_interconvertible_base_of`, `__is_array`, `__array_rank`, `__array_extent`, `__is_arithmetic`, `__is_complete_type`, `__is_compound`, `__is_const`, `__is_floating_point`, `__is_fundamental`, `__is_integral`, `__is_lvalue_reference`, `__is_member_function_pointer`, `__is_member_object_pointer`, `__is_member_pointer`, `__is_object`, `__is_pointer`, `__is_reference`, `__is_rvalue_reference`, `__is_scalar`, `__is_signed`, `__is_unsigned`, `__is_void`, and `__is_volatile`.
+
+### Bug Fixes
+
+* Fixed an issue in the taint tracking analysis where implicit reads were not allowed by default in sinks or additional taint steps that used flow states.

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

@@ -0,0 +1,3 @@
+## 0.4.1
+
+No user-facing changes.

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

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.3.5
+lastReleaseVersion: 0.4.1

cpp/ql/lib/experimental/semmle/code/cpp/dataflow/ProductFlow.qll

@@ -20,7 +20,8 @@ module ProductFlow {
      * `source1` and `source2` must belong to the same callable.
      */
     predicate isSourcePair(
-      DataFlow::Node source1, string state1, DataFlow::Node source2, string state2
+      DataFlow::Node source1, DataFlow::FlowState state1, DataFlow::Node source2,
+      DataFlow::FlowState state2
     ) {
       state1 = "" and
       state2 = "" and
@@ -49,6 +50,101 @@ module ProductFlow {
       this.isSinkPair(sink1, sink2)
     }
 
+    /**
+     * Holds if data flow through `node` is prohibited through the first projection of the product
+     * dataflow graph when the flow state is `state`.
+     */
+    predicate isBarrier1(DataFlow::Node node, DataFlow::FlowState state) {
+      this.isBarrier1(node) and state = ""
+    }
+
+    /**
+     * Holds if data flow through `node` is prohibited through the second projection of the product
+     * dataflow graph when the flow state is `state`.
+     */
+    predicate isBarrier2(DataFlow::Node node, DataFlow::FlowState state) {
+      this.isBarrier2(node) and state = ""
+    }
+
+    /**
+     * Holds if data flow through `node` is prohibited through the first projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrier1(DataFlow::Node node) { none() }
+
+    /**
+     * Holds if data flow through `node` is prohibited through the second projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrier2(DataFlow::Node node) { none() }
+
+    /**
+     * Holds if data flow out of `node` is prohibited in the first projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrierOut1(DataFlow::Node node) { none() }
+
+    /**
+     * Holds if data flow out of `node` is prohibited in the second projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrierOut2(DataFlow::Node node) { none() }
+
+    /*
+     * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps in
+     * the first projection of the product dataflow graph.
+     */
+
+    predicate isAdditionalFlowStep1(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+    /**
+     * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps in
+     * the first projection of the product dataflow graph.
+     *
+     * This step is only applicable in `state1` and updates the flow state to `state2`.
+     */
+    predicate isAdditionalFlowStep1(
+      DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+      DataFlow::FlowState state2
+    ) {
+      state1 instanceof DataFlow::FlowStateEmpty and
+      state2 instanceof DataFlow::FlowStateEmpty and
+      this.isAdditionalFlowStep1(node1, node2)
+    }
+
+    /**
+     * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps in
+     * the second projection of the product dataflow graph.
+     */
+    predicate isAdditionalFlowStep2(DataFlow::Node node1, DataFlow::Node node2) { none() }
+
+    /**
+     * Holds if data may flow from `node1` to `node2` in addition to the normal data-flow steps in
+     * the second projection of the product dataflow graph.
+     *
+     * This step is only applicable in `state1` and updates the flow state to `state2`.
+     */
+    predicate isAdditionalFlowStep2(
+      DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+      DataFlow::FlowState state2
+    ) {
+      state1 instanceof DataFlow::FlowStateEmpty and
+      state2 instanceof DataFlow::FlowStateEmpty and
+      this.isAdditionalFlowStep2(node1, node2)
+    }
+
+    /**
+     * Holds if data flow into `node` is prohibited in the first projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrierIn1(DataFlow::Node node) { none() }
+
+    /**
+     * Holds if data flow into `node` is prohibited in the second projection of the product
+     * dataflow graph.
+     */
+    predicate isBarrierIn2(DataFlow::Node node) { none() }
+
     predicate hasFlowPath(
       DataFlow::PathNode source1, DataFlow2::PathNode source2, DataFlow::PathNode sink1,
       DataFlow2::PathNode sink2
@@ -63,38 +159,78 @@ module ProductFlow {
     class Conf1 extends DataFlow::Configuration {
       Conf1() { this = "Conf1" }
 
-      override predicate isSource(DataFlow::Node source, string state) {
+      override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
         exists(Configuration conf | conf.isSourcePair(source, state, _, _))
       }
 
-      override predicate isSink(DataFlow::Node sink, string state) {
+      override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
         exists(Configuration conf | conf.isSinkPair(sink, state, _, _))
       }
+
+      override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+        exists(Configuration conf | conf.isBarrier1(node, state))
+      }
+
+      override predicate isBarrierOut(DataFlow::Node node) {
+        exists(Configuration conf | conf.isBarrierOut1(node))
+      }
+
+      override predicate isAdditionalFlowStep(
+        DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+        DataFlow::FlowState state2
+      ) {
+        exists(Configuration conf | conf.isAdditionalFlowStep1(node1, state1, node2, state2))
+      }
+
+      override predicate isBarrierIn(DataFlow::Node node) {
+        exists(Configuration conf | conf.isBarrierIn1(node))
+      }
     }
 
     class Conf2 extends DataFlow2::Configuration {
       Conf2() { this = "Conf2" }
 
-      override predicate isSource(DataFlow::Node source, string state) {
-        exists(Configuration conf, DataFlow::Node source1 |
-          conf.isSourcePair(source1, _, source, state) and
-          any(Conf1 c).hasFlow(source1, _)
+      override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+        exists(Configuration conf, DataFlow::PathNode source1 |
+          conf.isSourcePair(source1.getNode(), source1.getState(), source, state) and
+          any(Conf1 c).hasFlowPath(source1, _)
         )
       }
 
-      override predicate isSink(DataFlow::Node sink, string state) {
-        exists(Configuration conf, DataFlow::Node sink1 |
-          conf.isSinkPair(sink1, _, sink, state) and any(Conf1 c).hasFlow(_, sink1)
+      override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+        exists(Configuration conf, DataFlow::PathNode sink1 |
+          conf.isSinkPair(sink1.getNode(), sink1.getState(), sink, state) and
+          any(Conf1 c).hasFlowPath(_, sink1)
         )
       }
+
+      override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+        exists(Configuration conf | conf.isBarrier2(node, state))
+      }
+
+      override predicate isBarrierOut(DataFlow::Node node) {
+        exists(Configuration conf | conf.isBarrierOut2(node))
+      }
+
+      override predicate isAdditionalFlowStep(
+        DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+        DataFlow::FlowState state2
+      ) {
+        exists(Configuration conf | conf.isAdditionalFlowStep2(node1, state1, node2, state2))
+      }
+
+      override predicate isBarrierIn(DataFlow::Node node) {
+        exists(Configuration conf | conf.isBarrierIn2(node))
+      }
     }
   }
 
+  pragma[nomagic]
   private predicate reachableInterprocEntry(
     Configuration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2,
     DataFlow::PathNode node1, DataFlow2::PathNode node2
   ) {
-    conf.isSourcePair(node1.getNode(), _, node2.getNode(), _) and
+    conf.isSourcePair(node1.getNode(), node1.getState(), node2.getNode(), node2.getState()) and
     node1 = source1 and
     node2 = source2
     or
@@ -157,7 +293,7 @@ module ProductFlow {
   ) {
     exists(DataFlow::PathNode mid1, DataFlow2::PathNode mid2 |
       reachableInterprocEntry(conf, source1, source2, mid1, mid2) and
-      conf.isSinkPair(sink1.getNode(), _, sink2.getNode(), _) and
+      conf.isSinkPair(sink1.getNode(), sink1.getState(), sink2.getNode(), sink2.getState()) and
       localPathStep1*(mid1, sink1) and
       localPathStep2*(mid2, sink2)
     )

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -416,6 +416,21 @@ class SsaPhiNode extends Node, TSsaPhiNode {
   final override Location getLocationImpl() { result = phi.getBasicBlock().getLocation() }
 
   override string toStringImpl() { result = "Phi" }
+
+  /**
+   * Gets a node that is used as input to this phi node.
+   * `fromBackEdge` is true if data flows along a back-edge,
+   * and `false` otherwise.
+   */
+  final Node getAnInput(boolean fromBackEdge) {
+    localFlowStep(result, this) and
+    if phi.getBasicBlock().dominates(getBasicBlock(result))
+    then fromBackEdge = true
+    else fromBackEdge = false
+  }
+
+  /** Gets a node that is used as input to this phi node. */
+  final Node getAnInput() { result = this.getAnInput(_) }
 }
 
 /**

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -301,7 +301,12 @@ private predicate defToNode(Node nodeFrom, Def def) {
   nodeHasInstruction(nodeFrom, def.getDefiningInstruction(), def.getIndirectionIndex())
 }
 
-private predicate nodeToDefOrUse(Node nodeFrom, SsaDefOrUse defOrUse) {
+/**
+ * INTERNAL: Do not use.
+ *
+ * Holds if `nodeFrom` is the node that correspond to the definition or use `defOrUse`.
+ */
+predicate nodeToDefOrUse(Node nodeFrom, SsaDefOrUse defOrUse) {
   // Node -> Def
   defToNode(nodeFrom, defOrUse)
   or

cpp/ql/lib/experimental/semmle/code/cpp/semantic/SemanticExprSpecific.qll

@@ -7,6 +7,7 @@ private import semmle.code.cpp.ir.IR as IR
 private import Semantic
 private import experimental.semmle.code.cpp.rangeanalysis.Bound as IRBound
 private import semmle.code.cpp.controlflow.IRGuards as IRGuards
+private import semmle.code.cpp.ir.ValueNumbering
 
 module SemanticExprConfig {
   class Location = Cpp::Location;
@@ -120,7 +121,15 @@ module SemanticExprConfig {
 
   newtype TSsaVariable =
     TSsaInstruction(IR::Instruction instr) { instr.hasMemoryResult() } or
-    TSsaOperand(IR::Operand op) { op.isDefinitionInexact() }
+    TSsaOperand(IR::Operand op) { op.isDefinitionInexact() } or
+    TSsaPointerArithmeticGuard(IR::PointerArithmeticInstruction instr) {
+      exists(Guard g, IR::Operand use | use = instr.getAUse() |
+        g.comparesLt(use, _, _, _, _) or
+        g.comparesLt(_, use, _, _, _) or
+        g.comparesEq(use, _, _, _, _) or
+        g.comparesEq(_, use, _, _, _)
+      )
+    }
 
   class SsaVariable extends TSsaVariable {
     string toString() { none() }
@@ -129,6 +138,8 @@ module SemanticExprConfig {
 
     IR::Instruction asInstruction() { none() }
 
+    IR::PointerArithmeticInstruction asPointerArithGuard() { none() }
+
     IR::Operand asOperand() { none() }
   }
 
@@ -144,6 +155,18 @@ module SemanticExprConfig {
     final override IR::Instruction asInstruction() { result = instr }
   }
 
+  class SsaPointerArithmeticGuard extends SsaVariable, TSsaPointerArithmeticGuard {
+    IR::PointerArithmeticInstruction instr;
+
+    SsaPointerArithmeticGuard() { this = TSsaPointerArithmeticGuard(instr) }
+
+    final override string toString() { result = instr.toString() }
+
+    final override Location getLocation() { result = instr.getLocation() }
+
+    final override IR::PointerArithmeticInstruction asPointerArithGuard() { result = instr }
+  }
+
   class SsaOperand extends SsaVariable, TSsaOperand {
     IR::Operand op;
 
@@ -168,7 +191,11 @@ module SemanticExprConfig {
     )
   }
 
-  Expr getAUse(SsaVariable v) { result.(IR::LoadInstruction).getSourceValue() = v.asInstruction() }
+  Expr getAUse(SsaVariable v) {
+    result.(IR::LoadInstruction).getSourceValue() = v.asInstruction()
+    or
+    result = valueNumber(v.asPointerArithGuard()).getAnInstruction()
+  }
 
   SemType getSsaVariableType(SsaVariable v) {
     result = getSemanticType(v.asInstruction().getResultIRType())
@@ -208,7 +235,9 @@ module SemanticExprConfig {
 
     final override predicate hasRead(SsaVariable v) {
       exists(IR::Operand operand |
-        operand.getDef() = v.asInstruction() and
+        operand.getDef() = v.asInstruction() or
+        operand.getDef() = valueNumber(v.asPointerArithGuard()).getAnInstruction()
+      |
         not operand instanceof IR::PhiInputOperand and
         operand.getUse().getBlock() = block
       )
@@ -227,7 +256,9 @@ module SemanticExprConfig {
 
     final override predicate hasRead(SsaVariable v) {
       exists(IR::PhiInputOperand operand |
-        operand.getDef() = v.asInstruction() and
+        operand.getDef() = v.asInstruction() or
+        operand.getDef() = valueNumber(v.asPointerArithGuard()).getAnInstruction()
+      |
         operand.getPredecessorBlock() = pred and
         operand.getUse().getBlock() = succ
       )

cpp/ql/lib/experimental/semmle/code/cpp/semantic/SemanticSSA.qll

@@ -10,7 +10,7 @@ class SemSsaVariable instanceof Specific::SsaVariable {
 
   final Specific::Location getLocation() { result = super.getLocation() }
 
-  final SemLoadExpr getAUse() { result = Specific::getAUse(this) }
+  final SemExpr getAUse() { result = Specific::getAUse(this) }
 
   final SemType getType() { result = Specific::getSsaVariableType(this) }
 

cpp/ql/lib/qlpack.yml

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

cpp/ql/lib/semmle/code/cpp/commons/Strcat.qll

@@ -6,7 +6,7 @@ import cpp
  * A function that concatenates the string from its second argument
  * to the string from its first argument, for example `strcat`.
  */
-class StrcatFunction extends Function {
+deprecated class StrcatFunction extends Function {
   StrcatFunction() {
     getName() =
       [

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

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

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

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

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

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

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

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

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

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll

@@ -558,13 +558,16 @@ private predicate expectsContentEx(NodeEx n, Content c) {
 pragma[nomagic]
 private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) }
 
+pragma[nomagic]
+private predicate hasReadStep(Content c, Configuration config) { read(_, c, _, config) }
+
 pragma[nomagic]
 private predicate store(
   NodeEx node1, TypedContent tc, NodeEx node2, DataFlowType contentType, Configuration config
 ) {
   store(pragma[only_bind_into](node1.asNode()), tc, pragma[only_bind_into](node2.asNode()),
     contentType) and
-  read(_, tc.getContent(), _, config) and
+  hasReadStep(tc.getContent(), config) and
   stepFilter(node1, node2, config)
 }
 
@@ -598,13 +601,9 @@ private predicate hasSinkCallCtx(Configuration config) {
 }
 
 private module Stage1 implements StageSig {
-  class ApApprox = Unit;
-
   class Ap = Unit;
 
-  class ApOption = Unit;
-
-  class Cc = boolean;
+  private class Cc = boolean;
 
   /* Begin: Stage 1 logic. */
   /**
@@ -613,7 +612,7 @@ private module Stage1 implements StageSig {
    * The Boolean `cc` records whether the node is reached through an
    * argument in a call.
    */
-  predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
+  private predicate fwdFlow(NodeEx node, Cc cc, Configuration config) {
     sourceNode(node, _, config) and
     if hasSourceCallCtx(config) then cc = true else cc = false
     or
@@ -753,7 +752,7 @@ private module Stage1 implements StageSig {
    * the enclosing callable in order to reach a sink.
    */
   pragma[nomagic]
-  predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
+  private predicate revFlow(NodeEx node, boolean toReturn, Configuration config) {
     revFlow0(node, toReturn, config) and
     fwdFlow(node, config)
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -267,9 +267,6 @@ Instruction getSourceAddressFromNode(Node node) {
   result = getSourceAddress(node.asOperand().(SideEffectOperand).getUse())
 }
 
-/** Gets the source value of `instr` if it's an instruction that behaves like a `LoadInstruction`. */
-Instruction getSourceValue(Instruction instr) { result = getSourceValueOperand(instr).getDef() }
-
 /**
  * Gets the operand that represents the source value of `instr` if it's an instruction
  * that behaves like a `LoadInstruction`.

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

@@ -143,16 +143,6 @@ private module Cached {
     )
   }
 
-  cached
-  Instruction getRegisterOperandDefinition(Instruction instruction, RegisterOperandTag tag) {
-    exists(OldInstruction oldInstruction, OldIR::RegisterOperand oldOperand |
-      oldInstruction = getOldInstruction(instruction) and
-      oldOperand = oldInstruction.getAnOperand() and
-      tag = oldOperand.getOperandTag() and
-      result = getNewInstruction(oldOperand.getAnyDef())
-    )
-  }
-
   pragma[noopt]
   private predicate hasMemoryOperandDefinition(
     OldInstruction oldInstruction, OldIR::NonPhiMemoryOperand oldOperand, Overlap overlap,

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

@@ -256,12 +256,6 @@ CppType getInstructionOperandType(Instruction instruction, TypedOperandTag tag)
         .getInstructionMemoryOperandType(getInstructionTag(instruction), tag)
 }
 
-Instruction getPhiOperandDefinition(
-  PhiInstruction instruction, IRBlock predecessorBlock, Overlap overlap
-) {
-  none()
-}
-
 Instruction getPhiInstructionBlockStart(PhiInstruction instr) { none() }
 
 Instruction getInstructionSuccessor(Instruction instruction, EdgeKind kind) {

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

@@ -143,16 +143,6 @@ private module Cached {
     )
   }
 
-  cached
-  Instruction getRegisterOperandDefinition(Instruction instruction, RegisterOperandTag tag) {
-    exists(OldInstruction oldInstruction, OldIR::RegisterOperand oldOperand |
-      oldInstruction = getOldInstruction(instruction) and
-      oldOperand = oldInstruction.getAnOperand() and
-      tag = oldOperand.getOperandTag() and
-      result = getNewInstruction(oldOperand.getAnyDef())
-    )
-  }
-
   pragma[noopt]
   private predicate hasMemoryOperandDefinition(
     OldInstruction oldInstruction, OldIR::NonPhiMemoryOperand oldOperand, Overlap overlap,

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

@@ -205,57 +205,149 @@ private predicate deconstructSizeExpr(Expr sizeExpr, Expr lengthExpr, int sizeof
   sizeof = 1
 }
 
+/** A `Function` that is a call target of an allocation. */
+private signature class CallAllocationExprTarget extends Function;
+
 /**
- * An allocation expression that is a function call, such as call to `malloc`.
+ * This module abstracts over the type of allocation call-targets and provides a
+ * class `CallAllocationExprImpl` which contains the implementation of the various
+ * predicates required by the `Allocation` class.
+ *
+ * This module is then instantiated for two types of allocation call-targets:
+ * - `AllocationFunction`: Functions that we've explicitly modeled as functions that
+ * perform allocations (i.e., `malloc`).
+ * - `HeuristicAllocationFunction`: Functions that we deduce as behaving like an allocation
+ * function using various heuristics.
  */
-private class CallAllocationExpr extends AllocationExpr, FunctionCall {
-  AllocationFunction target;
+private module CallAllocationExprBase<CallAllocationExprTarget Target> {
+  /** A module that contains the collection of member-predicates required on `Target`. */
+  signature module Param {
+    /**
+     * Gets the index of the input pointer argument to be reallocated, if
+     * this is a `realloc` function.
+     */
+    int getReallocPtrArg(Target target);
 
-  CallAllocationExpr() {
-    target = this.getTarget() and
-    // realloc(ptr, 0) only frees the pointer
-    not (
-      exists(target.getReallocPtrArg()) and
-      this.getArgument(target.getSizeArg()).getValue().toInt() = 0
-    ) and
-    // these are modeled directly (and more accurately), avoid duplication
-    not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
+    /**
+     * Gets the index of the argument for the allocation size, if any. The actual
+     * allocation size is the value of this argument multiplied by the result of
+     * `getSizeMult()`, in bytes.
+     */
+    int getSizeArg(Target target);
+
+    /**
+     * Gets the index of an argument that multiplies the allocation size given
+     * by `getSizeArg`, if any.
+     */
+    int getSizeMult(Target target);
+
+    /**
+     * Holds if this allocation requires a
+     * corresponding deallocation of some sort (most do, but `alloca` for example
+     * does not). If it is unclear, we default to no (for example a placement `new`
+     * allocation may or may not require a corresponding `delete`).
+     */
+    predicate requiresDealloc(Target target);
   }
 
-  override Expr getSizeExpr() {
-    exists(Expr sizeExpr | sizeExpr = this.getArgument(target.getSizeArg()) |
-      if exists(target.getSizeMult())
-      then result = sizeExpr
-      else
-        exists(Expr lengthExpr |
-          deconstructSizeExpr(sizeExpr, lengthExpr, _) and
-          result = lengthExpr
+  /**
+   * A module that abstracts over a collection of predicates in
+   * the `Param` module). This should really be member-predicates
+   * on `CallAllocationExprTarget`, but we cannot yet write this in QL.
+   */
+  module With<Param P> {
+    private import P
+
+    /**
+     * An allocation expression that is a function call, such as call to `malloc`.
+     */
+    class CallAllocationExprImpl instanceof FunctionCall {
+      Target target;
+
+      CallAllocationExprImpl() {
+        target = this.getTarget() and
+        // realloc(ptr, 0) only frees the pointer
+        not (
+          exists(getReallocPtrArg(target)) and
+          this.getArgument(getSizeArg(target)).getValue().toInt() = 0
+        ) and
+        // these are modeled directly (and more accurately), avoid duplication
+        not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
+      }
+
+      string toString() { result = super.toString() }
+
+      Expr getSizeExprImpl() {
+        exists(Expr sizeExpr | sizeExpr = super.getArgument(getSizeArg(target)) |
+          if exists(getSizeMult(target))
+          then result = sizeExpr
+          else
+            exists(Expr lengthExpr |
+              deconstructSizeExpr(sizeExpr, lengthExpr, _) and
+              result = lengthExpr
+            )
         )
-    )
+      }
+
+      int getSizeMultImpl() {
+        // malloc with multiplier argument that is a constant
+        result = super.getArgument(getSizeMult(target)).getValue().toInt()
+        or
+        // malloc with no multiplier argument
+        not exists(getSizeMult(target)) and
+        deconstructSizeExpr(super.getArgument(getSizeArg(target)), _, result)
+      }
+
+      int getSizeBytesImpl() {
+        result = this.getSizeExprImpl().getValue().toInt() * this.getSizeMultImpl()
+      }
+
+      Expr getReallocPtrImpl() { result = super.getArgument(getReallocPtrArg(target)) }
+
+      Type getAllocatedElementTypeImpl() {
+        result =
+          super.getFullyConverted().getType().stripTopLevelSpecifiers().(PointerType).getBaseType() and
+        not result instanceof VoidType
+      }
+
+      predicate requiresDeallocImpl() { requiresDealloc(target) }
+    }
+  }
+}
+
+private module CallAllocationExpr {
+  private module Param implements CallAllocationExprBase<AllocationFunction>::Param {
+    int getReallocPtrArg(AllocationFunction f) { result = f.getReallocPtrArg() }
+
+    int getSizeArg(AllocationFunction f) { result = f.getSizeArg() }
+
+    int getSizeMult(AllocationFunction f) { result = f.getSizeMult() }
+
+    predicate requiresDealloc(AllocationFunction f) { f.requiresDealloc() }
   }
 
-  override int getSizeMult() {
-    // malloc with multiplier argument that is a constant
-    result = this.getArgument(target.getSizeMult()).getValue().toInt()
-    or
-    // malloc with no multiplier argument
-    not exists(target.getSizeMult()) and
-    deconstructSizeExpr(this.getArgument(target.getSizeArg()), _, result)
+  /**
+   * A class that provides the implementation of `AllocationExpr` for an allocation
+   * that calls an `AllocationFunction`.
+   */
+  private class Base =
+    CallAllocationExprBase<AllocationFunction>::With<Param>::CallAllocationExprImpl;
+
+  class CallAllocationExpr extends AllocationExpr, Base {
+    override Expr getSizeExpr() { result = super.getSizeExprImpl() }
+
+    override int getSizeMult() { result = super.getSizeMultImpl() }
+
+    override Type getAllocatedElementType() { result = super.getAllocatedElementTypeImpl() }
+
+    override predicate requiresDealloc() { super.requiresDeallocImpl() }
+
+    override int getSizeBytes() { result = super.getSizeBytesImpl() }
+
+    override Expr getReallocPtr() { result = super.getReallocPtrImpl() }
+
+    override string toString() { result = AllocationExpr.super.toString() }
   }
-
-  override int getSizeBytes() {
-    result = this.getSizeExpr().getValue().toInt() * this.getSizeMult()
-  }
-
-  override Expr getReallocPtr() { result = this.getArgument(target.getReallocPtrArg()) }
-
-  override Type getAllocatedElementType() {
-    result =
-      this.getFullyConverted().getType().stripTopLevelSpecifiers().(PointerType).getBaseType() and
-    not result instanceof VoidType
-  }
-
-  override predicate requiresDealloc() { target.requiresDealloc() }
 }
 
 /**
@@ -294,3 +386,99 @@ private class NewArrayAllocationExpr extends AllocationExpr, NewArrayExpr {
 
   override predicate requiresDealloc() { not exists(this.getPlacementPointer()) }
 }
+
+private module HeuristicAllocation {
+  /** A class that maps an `AllocationExpr` to an `HeuristicAllocationExpr`. */
+  private class HeuristicAllocationModeled extends HeuristicAllocationExpr instanceof AllocationExpr {
+    override Expr getSizeExpr() { result = AllocationExpr.super.getSizeExpr() }
+
+    override int getSizeMult() { result = AllocationExpr.super.getSizeMult() }
+
+    override int getSizeBytes() { result = AllocationExpr.super.getSizeBytes() }
+
+    override Expr getReallocPtr() { result = AllocationExpr.super.getReallocPtr() }
+
+    override Type getAllocatedElementType() {
+      result = AllocationExpr.super.getAllocatedElementType()
+    }
+
+    override predicate requiresDealloc() { AllocationExpr.super.requiresDealloc() }
+  }
+
+  /** A class that maps an `AllocationFunction` to an `HeuristicAllocationFunction`. */
+  private class HeuristicAllocationFunctionModeled extends HeuristicAllocationFunction instanceof AllocationFunction {
+    override int getSizeArg() { result = AllocationFunction.super.getSizeArg() }
+
+    override int getSizeMult() { result = AllocationFunction.super.getSizeMult() }
+
+    override int getReallocPtrArg() { result = AllocationFunction.super.getReallocPtrArg() }
+
+    override predicate requiresDealloc() { AllocationFunction.super.requiresDealloc() }
+  }
+
+  private int getAnUnsignedParameter(Function f) {
+    f.getParameter(result).getUnspecifiedType().(IntegralType).isUnsigned()
+  }
+
+  private int getAPointerParameter(Function f) {
+    f.getParameter(result).getUnspecifiedType() instanceof PointerType
+  }
+
+  /**
+   * A class that uses heuristics to find additional allocation functions. The required are as follows:
+   * 1. The word `alloc` must appear in the function name
+   * 2. The function must return a pointer type
+   * 3. There must be a unique parameter of unsigned integral type.
+   */
+  private class HeuristicAllocationFunctionByName extends HeuristicAllocationFunction instanceof Function {
+    int sizeArg;
+
+    HeuristicAllocationFunctionByName() {
+      Function.super.getName().matches("%alloc%") and
+      Function.super.getUnspecifiedType() instanceof PointerType and
+      sizeArg = unique( | | getAnUnsignedParameter(this))
+    }
+
+    override int getSizeArg() { result = sizeArg }
+
+    override int getReallocPtrArg() {
+      Function.super.getName().matches("%realloc%") and
+      result = unique( | | getAPointerParameter(this))
+    }
+
+    override predicate requiresDealloc() { none() }
+  }
+
+  private module Param implements CallAllocationExprBase<HeuristicAllocationFunction>::Param {
+    int getReallocPtrArg(HeuristicAllocationFunction f) { result = f.getReallocPtrArg() }
+
+    int getSizeArg(HeuristicAllocationFunction f) { result = f.getSizeArg() }
+
+    int getSizeMult(HeuristicAllocationFunction f) { result = f.getSizeMult() }
+
+    predicate requiresDealloc(HeuristicAllocationFunction f) { f.requiresDealloc() }
+  }
+
+  /**
+   * A class that provides the implementation of `AllocationExpr` for an allocation
+   * that calls an `HeuristicAllocationFunction`.
+   */
+  private class Base =
+    CallAllocationExprBase<HeuristicAllocationFunction>::With<Param>::CallAllocationExprImpl;
+
+  private class CallAllocationExpr extends HeuristicAllocationExpr, Base {
+    override Expr getSizeExpr() { result = super.getSizeExprImpl() }
+
+    override int getSizeMult() { result = super.getSizeMultImpl() }
+
+    override Type getAllocatedElementType() { result = super.getAllocatedElementTypeImpl() }
+
+    override predicate requiresDealloc() { super.requiresDeallocImpl() }
+
+    override int getSizeBytes() { result = super.getSizeBytesImpl() }
+
+    override Expr getReallocPtr() { result = super.getReallocPtrImpl() }
+
+    override string toString() { result = HeuristicAllocationExpr.super.toString() }
+  }
+}

cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll

@@ -113,3 +113,84 @@ class OperatorNewAllocationFunction extends AllocationFunction {
     result = 1
   }
 }
+
+/**
+ * An expression that _might_ allocate memory.
+ *
+ * Unlike `AllocationExpr`, this class uses heuristics (such as a call target's
+ * name and parameters) to include additional expressions.
+ */
+abstract class HeuristicAllocationExpr extends Expr {
+  /**
+   * Gets an expression for the allocation size, if any. The actual allocation
+   * size is the value of this expression multiplied by the result of
+   * `getSizeMult()`, in bytes.
+   */
+  Expr getSizeExpr() { none() }
+
+  /**
+   * Gets a constant multiplier for the allocation size given by `getSizeExpr`,
+   * in bytes.
+   */
+  int getSizeMult() { none() }
+
+  /**
+   * Gets the size of this allocation in bytes, if it is a fixed size and that
+   * size can be determined.
+   */
+  int getSizeBytes() { none() }
+
+  /**
+   * Gets the expression for the input pointer argument to be reallocated, if
+   * this is a `realloc` function.
+   */
+  Expr getReallocPtr() { none() }
+
+  /**
+   * Gets the type of the elements that are allocated, if it can be determined.
+   */
+  Type getAllocatedElementType() { none() }
+
+  /**
+   * Whether or not this allocation requires a corresponding deallocation of
+   * some sort (most do, but `alloca` for example does not).  If it is unclear,
+   * we default to no (for example a placement `new` allocation may or may not
+   * require a corresponding `delete`).
+   */
+  predicate requiresDealloc() { any() }
+}
+
+/**
+ * An function that _might_ allocate memory.
+ *
+ * Unlike `AllocationFunction`, this class uses heuristics (such as the function's
+ * name and its parameters) to include additional functions.
+ */
+abstract class HeuristicAllocationFunction extends Function {
+  /**
+   * Gets the index of the argument for the allocation size, if any. The actual
+   * allocation size is the value of this argument multiplied by the result of
+   * `getSizeMult()`, in bytes.
+   */
+  int getSizeArg() { none() }
+
+  /**
+   * Gets the index of an argument that multiplies the allocation size given by
+   * `getSizeArg`, if any.
+   */
+  int getSizeMult() { none() }
+
+  /**
+   * Gets the index of the input pointer argument to be reallocated, if this
+   * is a `realloc` function.
+   */
+  int getReallocPtrArg() { none() }
+
+  /**
+   * Whether or not this allocation requires a corresponding deallocation of
+   * some sort (most do, but `alloca` for example does not).  If it is unclear,
+   * we default to no (for example a placement `new` allocation may or may not
+   * require a corresponding `delete`).
+   */
+  predicate requiresDealloc() { any() }
+}

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

@@ -15,4 +15,4 @@ where
   c.fromSource() and
   c.isTopLevel() and
   c.getParentScope() instanceof GlobalNamespace
-select c, "This class is not declared in any namespace"
+select c, "This class is not declared in any namespace."

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

@@ -16,4 +16,4 @@ where
   t.fromSource() and
   n = t.getMetrics().getEfferentSourceCoupling() and
   n > 10
-select t as class_, "This class has too many dependencies (" + n.toString() + ")"
+select t as class_, "This class has too many dependencies (" + n.toString() + ")."

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

@@ -17,4 +17,4 @@ where
   n = f.getMetrics().getNumberOfCalls() and
   n > 99 and
   not f.isMultiplyDefined()
-select f as function, "This function makes too many calls (" + n.toString() + ")"
+select f as function, "This function makes too many calls (" + n.toString() + ")."

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

@@ -18,4 +18,4 @@ where
   f.getMetrics().getNumberOfParameters() > 15
 select f,
   "This function has too many parameters (" + f.getMetrics().getNumberOfParameters().toString() +
-    ")"
+    ")."

cpp/ql/src/Best Practices/Likely Errors/Slicing.ql

@@ -21,5 +21,5 @@ where
     rhsType.getAMember() = m and
     not m.(VirtualFunction).isPure()
   ) // add additional checks for concrete members in in-between supertypes
-select e, "This assignment expression slices from type $@ to $@", rhsType, rhsType.getName(),
+select e, "This assignment expression slices from type $@ to $@.", rhsType, rhsType.getName(),
   lhsType, lhsType.getName()

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

@@ -72,18 +72,6 @@ predicate floatTrivial(Literal lit) {
 
 predicate charLiteral(Literal lit) { lit instanceof CharLiteral }
 
-Type literalType(Literal literal) { result = literal.getType() }
-
-predicate stringType(DerivedType t) {
-  t.getBaseType() instanceof CharType
-  or
-  exists(SpecifiedType constCharType |
-    t.getBaseType() = constCharType and
-    constCharType.isConst() and
-    constCharType.getBaseType() instanceof CharType
-  )
-}
-
 predicate numberType(Type t) { t instanceof FloatingPointType or t instanceof IntegralType }
 
 predicate stringLiteral(Literal literal) { literal instanceof StringLiteral }

cpp/ql/src/Best Practices/NVI.ql

@@ -18,4 +18,4 @@ where
   f.hasSpecifier("virtual") and
   f.getFile().fromSource() and
   not f instanceof Destructor
-select f, "Avoid having public virtual methods (NVI idiom)"
+select f, "Avoid having public virtual methods (NVI idiom)."

cpp/ql/src/Best Practices/NVIHub.ql

@@ -23,4 +23,4 @@ where
   fclass = f.getDeclaringType() and
   hubIndex = fclass.getMetrics().getAfferentCoupling() * fclass.getMetrics().getEfferentCoupling() and
   hubIndex > 100
-select f, "Avoid having public virtual methods (NVI idiom)"
+select f, "Avoid having public virtual methods (NVI idiom)."

cpp/ql/src/Best Practices/SwitchLongCase.ql

@@ -38,5 +38,5 @@ where
   sc = switch.getASwitchCase() and
   tooLong(sc) and
   switchCaseLength(sc, lines)
-select switch, "Switch has at least one case that is too long: $@", sc,
+select switch, "Switch has at least one case that is too long: $@.", sc,
   sc.getExpr().toString() + " (" + lines.toString() + " lines)"

cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql

@@ -58,4 +58,4 @@ where
   not exists(AsmStmt s | f = s.getEnclosingFunction()) and
   not v.getAnAttribute().getName() = "unused" and
   not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr may use `v`
-select v, "Variable " + v.getName() + " is not used"
+select v, "Variable " + v.getName() + " is not used."

cpp/ql/src/Best Practices/Unused Entities/UnusedStaticVariables.ql

@@ -27,4 +27,4 @@ where
   not declarationHasSideEffects(v) and
   not v.getAnAttribute().hasName("used") and
   not v.getAnAttribute().hasName("unused")
-select v, "Static variable " + v.getName() + " is never read"
+select v, "Static variable " + v.getName() + " is never read."

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,20 @@
+## 0.4.1
+
+### Minor Analysis Improvements
+
+* The alert message of many queries have been changed to better follow the style guide and make the message consistent with other languages.
+
+## 0.4.0
+
+### New Queries
+
+* Added a new medium-precision query, `cpp/missing-check-scanf`, which detects `scanf` output variables that are used without a proper return-value check to see that they were actually written. A variation of this query was originally contributed as an [experimental query by @ihsinme](https://github.com/github/codeql/pull/8246).
+
+### Minor Analysis Improvements
+
+* Modernizations from "Cleartext storage of sensitive information in buffer" (`cpp/cleartext-storage-buffer`) have been ported to the "Cleartext storage of sensitive information in file" (`cpp/cleartext-storage-file`), "Cleartext transmission of sensitive information" (`cpp/cleartext-transmission`) and "Cleartext storage of sensitive information in an SQLite database" (`cpp/cleartext-storage-database`) queries. These changes may result in more correct results and fewer false positive results from these queries.
+* The alert message of many queries have been changed to make the message consistent with other languages.
+
 ## 0.3.4
 
 ## 0.3.3

cpp/ql/src/Critical/DeadCodeCondition.ql

@@ -64,5 +64,5 @@ where
   ) and
   (if context = test then testresult = "succeed" else testresult = "fail")
 select cond,
-  "Variable '" + v.getName() + "' is always " + context + " here, this check will always " +
-    testresult + "."
+  "Variable '" + v.getName() + "' is always " + context + ", this check will always " + testresult +
+    "."

cpp/ql/src/Critical/DescriptorNeverClosed.ql

@@ -29,4 +29,4 @@ from Expr alloc
 where
   allocateDescriptorCall(alloc) and
   not exists(ClosedExpr closed | closed.pointsTo() = alloc)
-select alloc, "This file descriptor is never closed"
+select alloc, "This file descriptor is never closed."

cpp/ql/src/Critical/FileMayNotBeClosed.ql

@@ -164,4 +164,4 @@ where
     fopenVariableReaches(v, def, ret) and
     ret.getAChild*() = v.getAnAccess()
   )
-select def, "The file opened here may not be closed at $@.", ret, "this exit point"
+select def, "This file may not be closed at $@.", ret, "this exit point"

cpp/ql/src/Critical/FileNeverClosed.ql

@@ -14,4 +14,4 @@ import FileClosed
 
 from Expr alloc
 where fopenCall(alloc) and not fopenCallMayBeClosed(alloc)
-select alloc, "The file is never closed"
+select alloc, "The file is never closed."

cpp/ql/src/Critical/InconsistentNullnessTesting.ql

@@ -27,4 +27,4 @@ where
     definitionUsePair(v, other, unchecked)
   )
 select unchecked,
-  "This dereference is not guarded by a non-null check, whereas other dereferences are guarded"
+  "This dereference is not guarded by a non-null check, whereas other dereferences are guarded."

cpp/ql/src/Critical/LateNegativeTest.ql

@@ -49,4 +49,4 @@ where
 select dangerous,
   "Variable '" + v.getName() +
     "' is used as an array-offset before it is tested for being negative (test on line " +
-    check.getLocation().getStartLine().toString() + "). "
+    check.getLocation().getStartLine().toString() + ")."

cpp/ql/src/Critical/MemoryMayNotBeFreed.ql

@@ -190,4 +190,4 @@ where
     allocatedVariableReaches(v, def, ret) and
     ret.getAChild*() = v.getAnAccess()
   )
-select def, "The memory allocated here may not be released at $@.", ret, "this exit point"
+select def, "This memory allocation may not be released at $@.", ret, "this exit point"

cpp/ql/src/Critical/MemoryNeverFreed.ql

@@ -16,4 +16,4 @@ from AllocationExpr alloc
 where
   alloc.requiresDealloc() and
   not allocMayBeFreed(alloc)
-select alloc, "This memory is never freed"
+select alloc, "This memory is never freed."

cpp/ql/src/Critical/MissingCheckScanf.ql

@@ -117,6 +117,6 @@ where
   output.getCall() = call and
   output.hasGuardedAccess(access, false)
 select access,
-  "$@ is read here, but may not have been written. " +
+  "This variable is read, but may not have been written. " +
     "It should be guarded by a check that the $@ returns at least " +
-    output.getMinimumGuardConstant() + ".", access, access.toString(), call, call.toString()
+    output.getMinimumGuardConstant() + ".", call, call.toString()

cpp/ql/src/Critical/NewArrayDeleteMismatch.ql

@@ -14,4 +14,4 @@ from Expr alloc, Expr free, Expr freed
 where
   allocReaches(freed, alloc, "new[]") and
   freeExprOrIndirect(free, freed, "delete")
-select free, "This memory may have been allocated with '$@', not 'new'.", alloc, "new[]"
+select free, "This memory may have been allocated with $@, not 'new'.", alloc, "new[]"

cpp/ql/src/Critical/NewDeleteArrayMismatch.ql

@@ -14,4 +14,4 @@ from Expr alloc, Expr free, Expr freed
 where
   allocReaches(freed, alloc, "new") and
   freeExprOrIndirect(free, freed, "delete[]")
-select free, "This memory may have been allocated with '$@', not 'new[]'.", alloc, "new"
+select free, "This memory may have been allocated with $@, not 'new[]'.", alloc, "new"

cpp/ql/src/Critical/Unused.ql

@@ -30,4 +30,4 @@ where
   not v.getType().getUnderlyingType() instanceof ReferenceType and
   not exists(ScopeUtilityClass util | def = util.getAUse()) and
   not def.isInMacroExpansion()
-select def, "Variable '" + v.getName() + "' is assigned a value that is never used"
+select def, "Variable '" + v.getName() + "' is assigned a value that is never used."

cpp/ql/src/Critical/UseAfterFree.ql

@@ -62,5 +62,5 @@ class UseAfterFreeReachability extends StackVariableReachability {
 
 from UseAfterFreeReachability r, StackVariable v, Expr free, Expr e
 where r.reaches(free, v, e)
-select e, "Memory pointed to by '" + v.getName().toString() + "' may have been previously freed $@",
-  free, "here"
+select e, "Memory pointed to by '" + v.getName().toString() + "' may have $@.", free,
+  "been previously freed"

cpp/ql/src/Diagnostics/SuccessfullyExtractedFiles.ql

@@ -3,6 +3,7 @@
  * @description Lists all files in the source code directory that were extracted without encountering a problem in the file.
  * @kind diagnostic
  * @id cpp/diagnostics/successfully-extracted-files
+ * @tags successfully-extracted-files
  */
 
 import cpp
@@ -12,4 +13,4 @@ from File f
 where
   not exists(ExtractionProblem e | e.getFile() = f) and
   exists(f.getRelativePath())
-select f, "File successfully extracted"
+select f, "File successfully extracted."

cpp/ql/src/Likely Bugs/Arithmetic/PointlessSelfComparison.ql

@@ -22,4 +22,4 @@ where
   not overflowTest(cmp) and
   not cmp.isFromTemplateInstantiation(_) and
   not isFromMacroDefinition(cmp)
-select cmp, "Self comparison."
+select cmp, "This expression compares an $@ to itself.", cmp.getLeftOperand(), "expression"

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

@@ -25,8 +25,11 @@ class CastToPointerArithFlow extends DataFlow::Configuration {
 
   override predicate isSource(DataFlow::Node node) {
     not node.asExpr() instanceof Conversion and
-    introducesNewField(node.asExpr().getType().(DerivedType).getBaseType(),
-      node.asExpr().getConversion*().getType().(DerivedType).getBaseType())
+    exists(Type baseType1, Type baseType2 |
+      hasBaseType(node.asExpr(), baseType1) and
+      hasBaseType(node.asExpr().getConversion*(), baseType2) and
+      introducesNewField(baseType1, baseType2)
+    )
   }
 
   override predicate isSink(DataFlow::Node node) {
@@ -35,6 +38,17 @@ class CastToPointerArithFlow extends DataFlow::Configuration {
   }
 }
 
+/**
+ * Holds if the type of `e` is a `DerivedType` with `base` as its base type.
+ *
+ * This predicate ensures that joins go from `e` to `base` instead
+ * of the other way around.
+ */
+pragma[inline]
+predicate hasBaseType(Expr e, Type base) {
+  pragma[only_bind_into](base) = e.getType().(DerivedType).getBaseType()
+}
+
 /**
  * `derived` has a (possibly indirect) base class of `base`, and at least one new
  * field has been introduced in the inheritance chain after `base`.
@@ -55,5 +69,5 @@ where
   cfg.hasFlowPath(source, sink) and
   source.getNode().asExpr().getFullyConverted().getUnspecifiedType() =
     sink.getNode().asExpr().getFullyConverted().getUnspecifiedType()
-select sink, source, sink,
-  "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here"
+select sink, source, sink, "This pointer arithmetic may be done with the wrong type because of $@.",
+  source, "this cast"

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

@@ -24,4 +24,4 @@ where
   va.getExplicitlyConverted().getType().getSize() > fct.getSize() and
   va.getTarget() = fi and
   not fct.getUnspecifiedType() instanceof BoolType
-select va, "Implicit downcast of bitfield $@", fi, fi.toString()
+select va, "Implicit downcast of bitfield $@.", fi, fi.toString()

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

@@ -49,5 +49,4 @@ where
   c.hasImplicitConversion() and
   not whiteListWrapped(c)
 select c,
-  "Return value of type " + t1.toString() + " is implicitly converted to " + t2.toString() +
-    " here."
+  "Return value of type " + t1.toString() + " is implicitly converted to " + t2.toString() + "."

cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

@@ -173,4 +173,4 @@ where
   not actual.getUnspecifiedType() instanceof ErroneousType
 select arg,
   "This argument should be of type '" + expected.getName() + "' but is of type '" +
-    actual.getUnspecifiedType().getName() + "'"
+    actual.getUnspecifiedType().getName() + "'."

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

@@ -17,5 +17,5 @@ import LeapYear
 from Expr source, Expr sink, PossibleYearArithmeticOperationCheckConfiguration config
 where config.hasFlow(DataFlow::exprNode(source), DataFlow::exprNode(sink))
 select sink,
-  "This arithmetic operation $@ uses a constant value of 365 ends up modifying the date/time located at $@, without considering leap year scenarios.",
-  source, source.toString(), sink, sink.toString()
+  "An arithmetic operation $@ that uses a constant value of 365 ends up modifying this date/time, without considering leap year scenarios.",
+  source, source.toString()

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

@@ -13,4 +13,4 @@ import NtohlArrayNoBound
 
 from NetworkToBufferSizeConfiguration bufConfig, DataFlow::Node source, DataFlow::Node sink
 where bufConfig.hasFlow(source, sink)
-select sink, "Unchecked use of data from network function $@", source, source.toString()
+select sink, "Unchecked use of data from network function $@.", source, source.toString()

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

@@ -81,4 +81,4 @@ from UninitialisedLocalReachability r, LocalVariable v, VariableAccess va
 where
   r.reaches(_, v, va) and
   not va = commonException()
-select va, "The variable $@ may not be initialized here.", v, v.getName()
+select va, "The variable $@ may not be initialized at this access.", v, v.getName()

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

@@ -399,5 +399,5 @@ where
   ) 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"
+select sink, source, sink, "Stack variable $@ escapes at $@ and is used after it has expired.", var,
+  var.toString(), store, "this store"

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

@@ -83,4 +83,4 @@ where
     c.getAMemberFunction().getAnOverriddenFunction() = call.getStaticCallTarget()
   )
 select call.getUnconvertedResultExpression(), source, sink,
-  "Call to pure virtual function during " + msg
+  "Call to pure virtual function during " + msg + "."

cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql

@@ -92,5 +92,6 @@ where
     isOptionSet(cc, BoostorgAsio::getShiftedSslOptionsNoTls1_2(), e) and
     msg = "no_tlsv1_2 was set"
   )
-select cc, "Usage of $@ with protocol $@ is not configured correctly: The option $@.", cc,
-  "boost::asio::ssl::context::context", protocolSource, protocolSource.toString(), e, msg
+select cc,
+  "This usage of 'boost::asio::ssl::context::context' with protocol $@ is not configured correctly: The option $@.",
+  protocolSource, protocolSource.toString(), e, msg

cpp/ql/src/Likely Bugs/RedundantNullCheckSimple.ql

@@ -67,5 +67,5 @@ where
   // the pointer was null. To follow this idea to its full generality, we
   // should also give an alert when `check` post-dominates `deref`.
   deref.getBlock() = dominator
-select checked, "This null check is redundant because the value is $@ in any case", deref,
-  "dereferenced here"
+select checked, "This null check is redundant because $@ in any case.", deref,
+  "the value is dereferenced"

cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -60,5 +60,5 @@ where
   taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
   isUserInput(taintSource, taintCause)
 select taintedArg, sourceNode, sinkNode,
-  "This argument to a file access function is derived from $@ and then passed to " + callChain,
+  "This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
   taintSource, "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -158,5 +158,5 @@ where
   concatResult = sinkNode.getState().(ExecState).getSndNode()
 select sinkAsArgumentIndirection(sinkNode.getNode()), sourceNode, sinkNode,
   "This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to "
-    + callChain, sourceNode, "user input (" + taintCause + ")", concatResult,
+    + callChain + ".", sourceNode, "user input (" + taintCause + ")", concatResult,
   concatResult.toString()

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

@@ -50,5 +50,5 @@ where
   taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
   isUserInput(taintSource, taintCause)
 select taintedArg, sourceNode, sinkNode,
-  "This argument to a SQL query function is derived from $@ and then passed to " + callChain,
+  "This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".",
   taintSource, "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql

@@ -34,5 +34,5 @@ where
   isProcessOperationExplanation(arg, processOperation) and
   taintedWithPath(source, arg, sourceNode, sinkNode)
 select arg, sourceNode, sinkNode,
-  "The value of this argument may come from $@ and is being passed to " + processOperation, source,
-  source.toString()
+  "The value of this argument may come from $@ and is being passed to " + processOperation + ".",
+  source, source.toString()

cpp/ql/src/Security/CWE/CWE-129/ImproperArrayIndexValidation.ql

@@ -116,6 +116,10 @@ class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
   }
 }
 
+/** Gets `str` where the first letter has been lowercased. */
+bindingset[str]
+string lowerFirst(string str) { result = str.prefix(1).toLowerCase() + str.suffix(1) }
+
 from
   ImproperArrayIndexValidationConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink,
   string sourceType
@@ -123,5 +127,5 @@ where
   conf.hasFlowPath(source, sink) and
   isFlowSource(source.getNode(), sourceType)
 select sink.getNode(), source, sink,
-  "$@ flows to here and is used in an array indexing expression, potentially causing an invalid access.",
-  source.getNode(), sourceType
+  "An array indexing expression depends on $@ that might be outside the bounds of the array.",
+  source.getNode(), lowerFirst(sourceType)

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

@@ -34,4 +34,4 @@ where
   isUserInput(userValue, cause)
 select arg, sourceNode, sinkNode,
   "The value of this argument may come from $@ and is being used as a formatting argument to " +
-    printfFunction, userValue, cause
+    printfFunction + ".", userValue, cause

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql

@@ -37,4 +37,4 @@ where
   isUserInput(userValue, cause)
 select arg, sourceNode, sinkNode,
   "The value of this argument may come from $@ and is being used as a formatting argument to " +
-    printfFunction, userValue, cause
+    printfFunction + ".", userValue, cause

cpp/ql/src/Security/CWE/CWE-170/ImproperNullTerminationTainted.ql

@@ -86,4 +86,5 @@ class TaintSource extends VariableAccess {
 
 from TaintSource source, VariableAccess sink
 where source.reaches(sink)
-select sink, "$@ flows to here and may not be null terminated.", source, "User-provided value"
+select sink, "String operation depends on a $@ that may not be null terminated.", source,
+  "user-provided value"

cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql

@@ -50,5 +50,5 @@ where
   op.getAnOperand() = e and
   missingGuard(op, e, effect)
 select e, sourceNode, sinkNode,
-  "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", origin,
-  "User-provided value"
+  "$@ flows to an operand of an arithmetic expression, potentially causing an " + effect + ".",
+  origin, "User-provided value"

cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

@@ -135,5 +135,5 @@ where
   sink.getNode().asExpr() = va and
   missingGuard(va, effect)
 select sink.getNode(), source, sink,
-  "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
-  getExpr(source.getNode()), "Uncontrolled value"
+  "Arithmetic expression depends on an $@, potentially causing an " + effect + ".",
+  getExpr(source.getNode()), "uncontrolled value"

cpp/ql/src/Security/CWE/CWE-190/ArithmeticWithExtremeValues.ql

@@ -75,5 +75,6 @@ where
     missingGuardAgainstOverflow(op, va) and effect = "overflow"
   ) and
   causeEffectCorrespond(cause, effect)
-select va, "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".",
+select va,
+  "$@ flows to an operand of an arithmetic expression, potentially causing an " + effect + ".",
   origin, "Extreme value"

cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql

@@ -36,5 +36,4 @@ where
   not inSystemMacroExpansion(use) and
   // Avoid double-counting: don't include all the conversions of `use`.
   not use instanceof Conversion
-select use, "$@ flows to here and is used in an expression which might " + kind + ".", origin,
-  "User-provided value"
+select use, "$@ flows an expression which might " + kind + ".", origin, "User-provided value"

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

@@ -99,5 +99,5 @@ where
   isFlowSource(source.getNode(), taintCause) and
   conf.hasFlowPath(source, sink) and
   allocSink(alloc, sink.getNode())
-select alloc, source, sink, "This allocation size is derived from $@ and might overflow",
+select alloc, source, sink, "This allocation size is derived from $@ and might overflow.",
   source.getNode(), "user input (" + taintCause + ")"

cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql

@@ -39,9 +39,13 @@ where
     isHresultBooleanConverted(e1, e2) and
     if e2.isImplicit()
     then
-      msg = "Implicit conversion from " + e1.getType().toString() + " to " + e2.getType().toString()
+      msg =
+        "Implicit conversion from " + e1.getType().toString() + " to " + e2.getType().toString() +
+          "."
     else
-      msg = "Explicit conversion from " + e1.getType().toString() + " to " + e2.getType().toString()
+      msg =
+        "Explicit conversion from " + e1.getType().toString() + " to " + e2.getType().toString() +
+          "."
   )
   or
   exists(ControlStructure ctls |
@@ -49,7 +53,7 @@ where
     e1.getType().(TypedefType).hasName("HRESULT") and
     not isHresultBooleanConverted(e1) and
     not ctls instanceof SwitchStmt and // not controlled by a boolean condition
-    msg = "Direct usage of a type " + e1.getType().toString() + " as a conditional expression"
+    msg = "Direct usage of a type " + e1.getType().toString() + " as a conditional expression."
   )
   or
   (
@@ -57,14 +61,14 @@ where
       e1.getType().(TypedefType).hasName("HRESULT") and
       msg =
         "Usage of a type " + e1.getType().toString() +
-          " as an argument of a binary logical operation"
+          " as an argument of a binary logical operation."
     )
     or
     exists(UnaryLogicalOperation ulop | ulop.getAnOperand() = e1 |
       e1.getType().(TypedefType).hasName("HRESULT") and
       msg =
         "Usage of a type " + e1.getType().toString() +
-          " as an argument of a unary logical operation"
+          " as an argument of a unary logical operation."
     ) and
     not isHresultBooleanConverted(e1)
   )

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql

@@ -44,5 +44,5 @@ where
   w.getASource() = sinkNode.getNode().asExpr() and
   dest = w.getDest()
 select w, sourceNode, sinkNode,
-  "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@", source,
+  "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@.", source,
   "user input (" + source.getSourceType() + ")"

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

@@ -73,5 +73,5 @@ where
   not isFileName(globalValueNumber(source)) and // file names are not passwords
   not exists(string convChar | convChar = w.getSourceConvChar(mid) | not convChar = ["s", "S"]) // ignore things written with other conversion characters
 select w, sourceNode, midNode,
-  "This write into file '" + dest.toString() + "' may contain unencrypted data from $@", source,
+  "This write into file '" + dest.toString() + "' may contain unencrypted data from $@.", source,
   "this source."

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -258,9 +258,9 @@ where
   then
     msg =
       "This operation transmits '" + sink.toString() +
-        "', which may contain unencrypted sensitive data from $@"
+        "', which may contain unencrypted sensitive data from $@."
   else
     msg =
       "This operation receives into '" + sink.toString() +
-        "', which may put unencrypted sensitive data into $@"
+        "', which may put unencrypted sensitive data into $@."
 select networkSendRecv, source, sink, msg, source, source.getNode().toString()

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

@@ -74,5 +74,6 @@ where
   config.hasFlowPath(source, sink) and
   source.getNode().asExpr() = sensitive and
   sqliteCall.getASource() = sink.getNode().asExpr()
-select sqliteCall, source, sink, "This SQLite call may store $@ in a non-encrypted SQLite database",
-  sensitive, "sensitive information"
+select sqliteCall, source, sink,
+  "This SQLite call may store $@ in a non-encrypted SQLite database.", sensitive,
+  "sensitive information"

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

@@ -97,4 +97,4 @@ from
 where
   config.hasFlowPath(source, sink) and
   str = source.getNode().asExpr()
-select str, source, sink, "A URL may be constructed with the HTTP protocol."
+select str, source, sink, "This URL may be constructed with the HTTP protocol."

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

@@ -10,12 +10,6 @@ private predicate reaches(ControlFlowNode a, ControlFlowNode b) = fastTC(success
 
 private predicate successor(ControlFlowNode a, ControlFlowNode b) { b = a.getASuccessor() }
 
-class WhitelistedCallsConfig extends string {
-  WhitelistedCallsConfig() { this = "config" }
-
-  abstract predicate isWhitelisted(Call c);
-}
-
 abstract class WhitelistedCall extends Call {
   override Function getTarget() { none() }
 }

cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql

@@ -44,5 +44,5 @@ where
   )
 select dest,
   "This pointer might have type $@ (size " + sourceBase.getSize() +
-    "), but the pointer arithmetic here is done with type " + destType + " (size " +
-    destBase.getSize() + ").", sourceLoc, sourceBase.toString()
+    "), but this pointer arithmetic is done with type " + destType + " (size " + destBase.getSize() +
+    ").", sourceLoc, sourceBase.toString()

cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingChar.ql

@@ -51,5 +51,5 @@ where
   destBase instanceof CharType
 select dest,
   "This pointer might have type $@ (size " + sourceBase.getSize() +
-    "), but the pointer arithmetic here is done with type " + destType + " (size " +
-    destBase.getSize() + ").", sourceLoc, sourceBase.toString()
+    "), but this pointer arithmetic is done with type " + destType + " (size " + destBase.getSize() +
+    ").", sourceLoc, sourceBase.toString()

cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql

@@ -26,4 +26,4 @@ where
   destBase instanceof VoidType
 select dest,
   "This pointer might have type $@ (size " + sourceBase.getSize() +
-    "), but the pointer arithmetic here is done with type void", sourceLoc, sourceBase.toString()
+    "), but this pointer arithmetic is done with type void.", sourceLoc, sourceBase.toString()

cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql

@@ -18,4 +18,4 @@ from FunctionCall call, Function target
 where
   call.getTarget() = target and
   target.hasGlobalOrStdName("gets")
-select call, "gets does not guard against buffer overflow"
+select call, "'gets' does not guard against buffer overflow."

cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql

@@ -16,7 +16,7 @@ import cpp
 predicate potentiallyDangerousFunction(Function f, string message) {
   exists(string name | f.hasGlobalName(name) |
     name = ["gmtime", "localtime", "ctime", "asctime"] and
-    message = "Call to " + name + " is potentially dangerous"
+    message = "Call to '" + name + "' is potentially dangerous."
   )
 }
 

cpp/ql/src/Security/CWE/CWE-732/OpenCallMissingModeArgument.ql

@@ -16,4 +16,4 @@ import FilePermissions
 from FileCreationWithOptionalModeExpr fc
 where not fc.hasModeArgument()
 select fc,
-  "A file is created here without providing a mode argument, which may leak bits from the stack."
+  "This creates a file without providing a mode argument, which may leak bits from the stack."