Merge pull request #13249 from github/redsun82/swift-hidden-ast

Swift: make only certain elements hideable in the AST

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

@@ -27,9 +27,9 @@ jobs:
         run: |
           gh api 'repos/${{github.repository}}/pulls/${{github.event.number}}/files' --paginate --jq 'any(.[].filename ; test("/change-notes/.*[.]md$"))' |
           grep true -c
-      - name: Fail if the change note filename doesn't match the expected format. The file name must be of the form 'YYYY-MM-DD.md' or 'YYYY-MM-DD-{title}.md', where '{title}' is arbitrary text.
+      - name: Fail if the change note filename doesn't match the expected format. The file name must be of the form 'YYYY-MM-DD.md', 'YYYY-MM-DD-{title}.md', where '{title}' is arbitrary text, or released/x.y.z.md for released change-notes
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          gh api 'repos/${{github.repository}}/pulls/${{github.event.number}}/files' --paginate --jq '[.[].filename | select(test("/change-notes/.*[.]md$"))] | all(test("/change-notes/[0-9]{4}-[0-9]{2}-[0-9]{2}.*[.]md$"))' |
-          grep true -c
+          gh api 'repos/${{github.repository}}/pulls/${{github.event.number}}/files' --paginate --jq '[.[].filename | select(test("/change-notes/.*[.]md$"))] | all(test("/change-notes/[0-9]{4}-[0-9]{2}-[0-9]{2}.*[.]md$") or test("/change-notes/released/[0-9]*[.][0-9]*[.][0-9]*[.]md$"))' |
+          grep true -c 

.github/workflows/swift.yml

@@ -16,6 +16,7 @@ on:
     branches:
       - main
       - rc/*
+      - codeql-cli-*
   push:
     paths:
       - "swift/**"
@@ -30,6 +31,7 @@ on:
     branches:
       - main
       - rc/*
+      - codeql-cli-*
 
 jobs:
   # not using a matrix as you cannot depend on a specific job in a matrix, and we want to start linux checks

.vscode/tasks.json

@@ -22,6 +22,22 @@
                 "command": "${config:python.pythonPath}",
             },
             "problemMatcher": []
+        },
+        {
+            "label": "Accept .expected changes from CI",
+            "type": "process",
+            // Non-Windows OS will usually have Python 3 already installed at /usr/bin/python3.
+            "command": "python3",
+            "args": [
+                "misc/scripts/accept-expected-changes-from-ci.py"
+            ],
+            "group": "build",
+            "windows": {
+                // On Windows, use whatever Python interpreter is configured for this workspace. The default is
+                // just `python`, so if Python is already on the path, this will find it.
+                "command": "${config:python.pythonPath}",
+            },
+            "problemMatcher": []
         }
     ]
-}
+}

CODEOWNERS

@@ -40,3 +40,6 @@ WORKSPACE.bazel @github/codeql-ci-reviewers
 /.github/workflows/ql-for-ql-* @github/codeql-ql-for-ql-reviewers
 /.github/workflows/ruby-* @github/codeql-ruby
 /.github/workflows/swift.yml @github/codeql-swift
+
+# Misc
+/misc/scripts/accept-expected-changes-from-ci.py @RasmusWL

config/identical-files.json

@@ -47,7 +47,6 @@
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl2.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl3.qll",
     "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImpl4.qll",
-    "python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplForRegExp.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl1.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForHttpClientLibraries.qll",

cpp/ql/lib/CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ### Minor Analysis Improvements
 
+* The `StdNamespace` class now also includes all inline namespaces that are children of `std` namespace.
 * The new dataflow (`semmle.code.cpp.dataflow.new.DataFlow`) and taint-tracking libraries (`semmle.code.cpp.dataflow.new.TaintTracking`) now support tracking flow through static local variables.
 
 ## 0.7.1

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

@@ -11,4 +11,5 @@
 
 ### Minor Analysis Improvements
 
+* The `StdNamespace` class now also includes all inline namespaces that are children of `std` namespace.
 * The new dataflow (`semmle.code.cpp.dataflow.new.DataFlow`) and taint-tracking libraries (`semmle.code.cpp.dataflow.new.TaintTracking`) now support tracking flow through static local variables.

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

@@ -290,9 +290,9 @@ module ProductFlow {
       predicate isBarrierIn(DataFlow::Node node) { Config::isBarrierIn1(node) }
     }
 
-    module Flow1 = DataFlow::GlobalWithState<Config1>;
+    private module Flow1 = DataFlow::GlobalWithState<Config1>;
 
-    module Config2 implements DataFlow::StateConfigSig {
+    private module Config2 implements DataFlow::StateConfigSig {
       class FlowState = FlowState2;
 
       predicate isSource(DataFlow::Node source, FlowState state) {
@@ -322,27 +322,90 @@ module ProductFlow {
       predicate isBarrierIn(DataFlow::Node node) { Config::isBarrierIn2(node) }
     }
 
-    module Flow2 = DataFlow::GlobalWithState<Config2>;
+    private module Flow2 = DataFlow::GlobalWithState<Config2>;
+
+    private predicate isSourcePair(Flow1::PathNode node1, Flow2::PathNode node2) {
+      Config::isSourcePair(node1.getNode(), node1.getState(), node2.getNode(), node2.getState())
+    }
+
+    private predicate isSinkPair(Flow1::PathNode node1, Flow2::PathNode node2) {
+      Config::isSinkPair(node1.getNode(), node1.getState(), node2.getNode(), node2.getState())
+    }
+
+    pragma[assume_small_delta]
+    pragma[nomagic]
+    private predicate fwdReachableInterprocEntry(Flow1::PathNode node1, Flow2::PathNode node2) {
+      isSourcePair(node1, node2)
+      or
+      fwdIsSuccessor(_, _, node1, node2)
+    }
 
     pragma[nomagic]
-    private predicate reachableInterprocEntry(
-      Flow1::PathNode source1, Flow2::PathNode source2, Flow1::PathNode node1, Flow2::PathNode node2
+    private predicate fwdIsSuccessorExit(
+      Flow1::PathNode mid1, Flow2::PathNode mid2, Flow1::PathNode succ1, Flow2::PathNode succ2
     ) {
-      Config::isSourcePair(node1.getNode(), node1.getState(), node2.getNode(), node2.getState()) and
-      node1 = source1 and
-      node2 = source2
+      isSinkPair(mid1, mid2) and
+      succ1 = mid1 and
+      succ2 = mid2
       or
-      exists(
-        Flow1::PathNode midEntry1, Flow2::PathNode midEntry2, Flow1::PathNode midExit1,
-        Flow2::PathNode midExit2
-      |
-        reachableInterprocEntry(source1, source2, midEntry1, midEntry2) and
-        interprocEdgePair(midExit1, midExit2, node1, node2) and
-        localPathStep1*(midEntry1, midExit1) and
-        localPathStep2*(midEntry2, midExit2)
+      interprocEdgePair(mid1, mid2, succ1, succ2)
+    }
+
+    private predicate fwdIsSuccessor1(
+      Flow1::PathNode pred1, Flow2::PathNode pred2, Flow1::PathNode mid1, Flow2::PathNode mid2,
+      Flow1::PathNode succ1, Flow2::PathNode succ2
+    ) {
+      fwdReachableInterprocEntry(pred1, pred2) and
+      localPathStep1*(pred1, mid1) and
+      fwdIsSuccessorExit(pragma[only_bind_into](mid1), pragma[only_bind_into](mid2), succ1, succ2)
+    }
+
+    private predicate fwdIsSuccessor2(
+      Flow1::PathNode pred1, Flow2::PathNode pred2, Flow1::PathNode mid1, Flow2::PathNode mid2,
+      Flow1::PathNode succ1, Flow2::PathNode succ2
+    ) {
+      fwdReachableInterprocEntry(pred1, pred2) and
+      localPathStep2*(pred2, mid2) and
+      fwdIsSuccessorExit(pragma[only_bind_into](mid1), pragma[only_bind_into](mid2), succ1, succ2)
+    }
+
+    pragma[assume_small_delta]
+    private predicate fwdIsSuccessor(
+      Flow1::PathNode pred1, Flow2::PathNode pred2, Flow1::PathNode succ1, Flow2::PathNode succ2
+    ) {
+      exists(Flow1::PathNode mid1, Flow2::PathNode mid2 |
+        fwdIsSuccessor1(pred1, pred2, mid1, mid2, succ1, succ2) and
+        fwdIsSuccessor2(pred1, pred2, mid1, mid2, succ1, succ2)
       )
     }
 
+    pragma[assume_small_delta]
+    pragma[nomagic]
+    private predicate revReachableInterprocEntry(Flow1::PathNode node1, Flow2::PathNode node2) {
+      fwdReachableInterprocEntry(node1, node2) and
+      isSinkPair(node1, node2)
+      or
+      exists(Flow1::PathNode succ1, Flow2::PathNode succ2 |
+        revReachableInterprocEntry(succ1, succ2) and
+        fwdIsSuccessor(node1, node2, succ1, succ2)
+      )
+    }
+
+    private newtype TNodePair =
+      TMkNodePair(Flow1::PathNode node1, Flow2::PathNode node2) {
+        revReachableInterprocEntry(node1, node2)
+      }
+
+    private predicate pathSucc(TNodePair n1, TNodePair n2) {
+      exists(Flow1::PathNode n11, Flow2::PathNode n12, Flow1::PathNode n21, Flow2::PathNode n22 |
+        n1 = TMkNodePair(n11, n12) and
+        n2 = TMkNodePair(n21, n22) and
+        fwdIsSuccessor(n11, n12, n21, n22)
+      )
+    }
+
+    private predicate pathSuccPlus(TNodePair n1, TNodePair n2) = fastTC(pathSucc/2)(n1, n2)
+
     private predicate localPathStep1(Flow1::PathNode pred, Flow1::PathNode succ) {
       Flow1::PathGraph::edges(pred, succ) and
       pragma[only_bind_out](pred.getNode().getEnclosingCallable()) =
@@ -474,11 +537,14 @@ module ProductFlow {
     private predicate reachable(
       Flow1::PathNode source1, Flow2::PathNode source2, Flow1::PathNode sink1, Flow2::PathNode sink2
     ) {
-      exists(Flow1::PathNode mid1, Flow2::PathNode mid2 |
-        reachableInterprocEntry(source1, source2, mid1, mid2) and
-        Config::isSinkPair(sink1.getNode(), sink1.getState(), sink2.getNode(), sink2.getState()) and
-        localPathStep1*(mid1, sink1) and
-        localPathStep2*(mid2, sink2)
+      isSourcePair(source1, source2) and
+      isSinkPair(sink1, sink2) and
+      exists(TNodePair n1, TNodePair n2 |
+        n1 = TMkNodePair(source1, source2) and
+        n2 = TMkNodePair(sink1, sink2)
+      |
+        pathSuccPlus(n1, n2) or
+        n1 = n2
       )
     }
   }

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

@@ -230,8 +230,12 @@ class GlobalNamespace extends Namespace {
 }
 
 /**
- * The C++ `std::` namespace.
+ * The C++ `std::` namespace and its inline namespaces.
  */
 class StdNamespace extends Namespace {
-  StdNamespace() { this.hasName("std") and this.getParentNamespace() instanceof GlobalNamespace }
+  StdNamespace() {
+    this.hasName("std") and this.getParentNamespace() instanceof GlobalNamespace
+    or
+    this.isInline() and this.getParentNamespace() instanceof StdNamespace
+  }
 }

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

@@ -1699,7 +1699,28 @@ class AutoType extends TemplateParameter {
 
 private predicate suppressUnusedThis(Type t) { any() }
 
-/** A source code location referring to a type */
+/**
+ * A source code location referring to a user-defined type.
+ *
+ * Note that only _user-defined_ types have `TypeMention`s. In particular,
+ * built-in types, and derived types with built-in types as their base don't
+ * have any `TypeMention`s. For example, given
+ * ```cpp
+ * struct S { ... };
+ * void f(S s1, int i1) {
+ *   S s2;
+ *   S* s3;
+ *   S& s4 = s2;
+ *   decltype(s2) s5;
+ *
+ *   int i2;
+ *   int* i3;
+ *   int i4[10];
+ * }
+ * ```
+ * there will be a `TypeMention` for the mention of `S` at `S s1`, `S s2`, and `S& s4 = s2`,
+ * but not at `decltype(s2) s5`. Additionally, there will be no `TypeMention`s for `int`.
+ */
 class TypeMention extends Locatable, @type_mention {
   override string toString() { result = "type mention" }
 

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

@@ -657,24 +657,16 @@ private predicate indirectConversionFlowStep(Node nFrom, Node nTo) {
  * So this predicate recurses back along conversions and `PointerArithmeticInstruction`s to find the
  * first use that has provides use-use flow, and uses that target as the target of the `nodeFrom`.
  */
-private predicate adjustForPointerArith(
-  DefOrUse defOrUse, Node nodeFrom, UseOrPhi use, boolean uncertain
-) {
-  nodeFrom = any(PostUpdateNode pun).getPreUpdateNode() and
-  exists(Node adjusted |
-    indirectConversionFlowStep*(adjusted, nodeFrom) and
-    nodeToDefOrUse(adjusted, defOrUse, uncertain) and
+private predicate adjustForPointerArith(PostUpdateNode pun, UseOrPhi use) {
+  exists(DefOrUse defOrUse, Node adjusted |
+    indirectConversionFlowStep*(adjusted, pun.getPreUpdateNode()) and
+    nodeToDefOrUse(adjusted, defOrUse, _) and
     adjacentDefRead(defOrUse, use)
   )
 }
 
 private predicate ssaFlowImpl(SsaDefOrUse defOrUse, Node nodeFrom, Node nodeTo, boolean uncertain) {
-  // `nodeFrom = any(PostUpdateNode pun).getPreUpdateNode()` is implied by adjustedForPointerArith.
   exists(UseOrPhi use |
-    adjustForPointerArith(defOrUse, nodeFrom, use, uncertain) and
-    useToNode(use, nodeTo)
-    or
-    not nodeFrom = any(PostUpdateNode pun).getPreUpdateNode() and
     nodeToDefOrUse(nodeFrom, defOrUse, uncertain) and
     adjacentDefRead(defOrUse, use) and
     useToNode(use, nodeTo) and
@@ -719,14 +711,19 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) {
   )
 }
 
+private predicate isArgumentOfCallable(DataFlowCall call, ArgumentNode arg) {
+  arg.argumentOf(call, _)
+}
+
+/** Holds if there is def-use or use-use flow from `pun` to `nodeTo`. */
 predicate postUpdateFlow(PostUpdateNode pun, Node nodeTo) {
-  exists(Node preUpdate, Node nFrom, boolean uncertain, SsaDefOrUse defOrUse |
+  exists(UseOrPhi use, Node preUpdate |
+    adjustForPointerArith(pun, use) and
+    useToNode(use, nodeTo) and
     preUpdate = pun.getPreUpdateNode() and
-    ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain)
-  |
-    if uncertain = true
-    then preUpdate = [nFrom, getAPriorDefinition(defOrUse)]
-    else preUpdate = nFrom
+    not exists(DataFlowCall call |
+      isArgumentOfCallable(call, preUpdate) and isArgumentOfCallable(call, nodeTo)
+    )
   )
 }
 

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

@@ -414,7 +414,7 @@ private module HeuristicAllocation {
     int sizeArg;
 
     HeuristicAllocationFunctionByName() {
-      Function.super.getName().matches("%alloc%") and
+      Function.super.getName().matches(["%alloc%", "%Alloc%"]) and
       Function.super.getUnspecifiedType() instanceof PointerType and
       sizeArg = unique( | | getAnUnsignedParameter(this))
     }

cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql

@@ -47,7 +47,7 @@ VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
  * Holds if `(n, state)` pair represents the source of flow for the size
  * expression associated with `alloc`.
  */
-predicate hasSize(AllocationExpr alloc, DataFlow::Node n, int state) {
+predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
   exists(VariableAccess va, Expr size, int delta |
     size = alloc.getSizeExpr() and
     // Get the unique variable in a size expression like `x` in `malloc(x + 1)`.
@@ -78,6 +78,145 @@ predicate isSinkPairImpl(
   )
 }
 
+module ValidState {
+  /**
+   * In the `StringSizeConfig` configuration we use an integer as the flow state for the second
+   * projection of the dataflow graph. The integer represents an offset that is added to the
+   * size of the allocation. For example, given:
+   * ```cpp
+   * char* p = new char[size + 1];
+   * size += 1;
+   * memset(p, 0, size);
+   * ```
+   * the initial flow state is `1`. This represents the fact that `size + 1` is a valid bound
+   * for the size of the allocation pointed to by `p`. After updating the size using `+=`, the
+   * flow state changes to `0`, which represents the fact that `size + 0` is a valid bound for
+   * the allocation.
+   *
+   * So we need to compute a set of valid integers that represent the offset applied to the
+   * size. We do this in two steps:
+   * 1. We first perform the dataflow traversal that the second projection of the product-flow
+   * library will perform, and visit all the places where the size argument is modified.
+   * 2. Once that dataflow traversal is done, we accumulate the offsets added at each places
+   * where the offset is modified (see `validStateImpl`).
+   *
+   * Because we want to guarantee that each place where we modify the offset has a `PathNode`
+   * we "flip" a boolean flow state in each `isAdditionalFlowStep`. This ensures that the node
+   * has a corresponding `PathNode`.
+   */
+  private module ValidStateConfig implements DataFlow::StateConfigSig {
+    class FlowState = boolean;
+
+    predicate isSource(DataFlow::Node source, FlowState state) {
+      hasSize(_, source, _) and
+      state = false
+    }
+
+    predicate isSink(DataFlow::Node sink, FlowState state) {
+      isSinkPairImpl(_, _, sink, _, _) and
+      state = [false, true]
+    }
+
+    predicate isBarrier(DataFlow::Node node, FlowState state) { none() }
+
+    predicate isAdditionalFlowStep(
+      DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+    ) {
+      isAdditionalFlowStep2(node1, node2, _) and
+      state1 = [false, true] and
+      state2 = state1.booleanNot()
+    }
+
+    predicate includeHiddenNodes() { any() }
+  }
+
+  private import DataFlow::GlobalWithState<ValidStateConfig>
+
+  private predicate inLoop(PathNode n) { n.getASuccessor+() = n }
+
+  /**
+   * Holds if `value` is a possible offset for `n`.
+   *
+   * To ensure termination, we limit `value` to be in the
+   * range `[-2, 2]` if the node is part of a loop. Without
+   * this restriction we wouldn't terminate on an example like:
+   * ```cpp
+   * while(unknown()) { size++; }
+   * ```
+   */
+  private predicate validStateImpl(PathNode n, int value) {
+    // If the dataflow node depends recursively on itself we restrict the range.
+    (inLoop(n) implies value = [-2 .. 2]) and
+    (
+      // For the dataflow source we have an allocation such as `malloc(size + k)`,
+      // and the value of the flow-state is then `k`.
+      hasSize(_, n.getNode(), value)
+      or
+      // For a dataflow sink any `value` that is strictly smaller than the delta
+      // needs to be a valid flow-state. That is, for a snippet like:
+      // ```
+      // p = b ? new char[size] : new char[size + 1];
+      // memset(p, 0, size + 2);
+      // ```
+      // the valid flow-states at the `memset` must include the set `{0, 1}` since the
+      // flow-state at `new char[size]` is `0`, and the flow-state at `new char[size + 1]`
+      // is `1`.
+      //
+      // So we find a valid flow-state at the sink's predecessor, and use the definition
+      // of our sink predicate to compute the valid flow-states at the sink.
+      exists(int delta, PathNode n0 |
+        n0.getASuccessor() = n and
+        validStateImpl(n0, value) and
+        isSinkPairImpl(_, _, n.getNode(), delta, _) and
+        delta > value
+      )
+      or
+      // For a non-source and non-sink node there is two cases to consider.
+      // 1. A node where we have to update the flow-state, or
+      // 2. A node that doesn't update the flow-state.
+      //
+      // For case 1, we compute the new flow-state by adding the constant operand of the
+      // `AddInstruction` to the flow-state of any predecessor node.
+      // For case 2 we simply propagate the valid flow-states from the predecessor node to
+      // the next one.
+      exists(PathNode n0, DataFlow::Node node0, DataFlow::Node node, int value0 |
+        n0.getASuccessor() = n and
+        validStateImpl(n0, value0) and
+        node = n.getNode() and
+        node0 = n0.getNode()
+      |
+        exists(int delta |
+          isAdditionalFlowStep2(node0, node, delta) and
+          value0 = value + delta
+        )
+        or
+        not isAdditionalFlowStep2(node0, node, _) and
+        value = value0
+      )
+    )
+  }
+
+  predicate validState(DataFlow::Node n, int value) {
+    validStateImpl(any(PathNode pn | pn.getNode() = n), value)
+  }
+}
+
+import ValidState
+
+/**
+ * Holds if `node2` is a dataflow node that represents an addition of two operands `op1`
+ * and `op2` such that:
+ * 1. `node1` is the dataflow node that represents `op1`, and
+ * 2. the value of `op2` can be upper bounded by `delta.`
+ */
+predicate isAdditionalFlowStep2(DataFlow::Node node1, DataFlow::Node node2, int delta) {
+  exists(AddInstruction add, Operand op |
+    add.hasOperands(node1.asOperand(), op) and
+    semBounded(getSemanticExpr(op.getDef()), any(SemZeroBound zero), delta, true, _) and
+    node2.asInstruction() = add
+  )
+}
+
 module StringSizeConfig implements ProductFlow::StateConfigSig {
   class FlowState1 = Unit;
 
@@ -100,7 +239,7 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
     DataFlow::Node bufSink, FlowState1 state1, DataFlow::Node sizeSink, FlowState2 state2
   ) {
     exists(state1) and
-    state2 = [-32 .. 32] and // An arbitrary bound because we need to bound `state2`
+    validState(sizeSink, state2) and
     exists(int delta |
       isSinkPairImpl(_, bufSink, sizeSink, delta, _) and
       delta > state2
@@ -111,6 +250,10 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
 
   predicate isBarrier2(DataFlow::Node node, FlowState2 state) { none() }
 
+  predicate isBarrierOut2(DataFlow::Node node) {
+    node = any(DataFlow::SsaPhiNode phi).getAnInput(true)
+  }
+
   predicate isAdditionalFlowStep1(
     DataFlow::Node node1, FlowState1 state1, DataFlow::Node node2, FlowState1 state2
   ) {
@@ -120,14 +263,10 @@ module StringSizeConfig implements ProductFlow::StateConfigSig {
   predicate isAdditionalFlowStep2(
     DataFlow::Node node1, FlowState2 state1, DataFlow::Node node2, FlowState2 state2
   ) {
-    exists(AddInstruction add, Operand op, int delta, int s1, int s2 |
-      s1 = [-32 .. 32] and // An arbitrary bound because we need to bound `state`
-      state1 = s1 and
-      state2 = s2 and
-      add.hasOperands(node1.asOperand(), op) and
-      semBounded(getSemanticExpr(op.getDef()), any(SemZeroBound zero), delta, true, _) and
-      node2.asInstruction() = add and
-      s1 = s2 + delta
+    validState(node2, state2) and
+    exists(int delta |
+      isAdditionalFlowStep2(node1, node2, delta) and
+      state1 = state2 + delta
     )
   }
 }

cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

@@ -48,44 +48,23 @@ bindingset[b]
 pragma[inline_late]
 predicate bounded2(Instruction i, Instruction b, int delta) { boundedImpl(i, b, delta) }
 
-/**
- * Holds if the combination of `n` and `state` represents an appropriate
- * source for the expression `e` suitable for use-use flow.
- */
-private predicate hasSizeImpl(Expr e, DataFlow::Node n, int state) {
-  // The simple case: If the size is a variable access with no qualifier we can just use the
-  // dataflow node for that expression and no state.
-  exists(VariableAccess va |
-    va = e and
-    not va instanceof FieldAccess and
-    n.asConvertedExpr() = va.getFullyConverted() and
-    state = 0
-  )
-  or
-  // If the size is a choice between two expressions we allow both to be nodes representing the size.
-  exists(ConditionalExpr cond | cond = e | hasSizeImpl([cond.getThen(), cond.getElse()], n, state))
-  or
-  // If the size is an expression plus a constant, we pick the dataflow node of the expression and
-  // remember the constant in the state.
-  exists(Expr const, Expr nonconst |
-    e.(AddExpr).hasOperands(const, nonconst) and
-    state = const.getValue().toInt() and
-    hasSizeImpl(nonconst, n, _)
-  )
-  or
-  exists(Expr const, Expr nonconst |
-    e.(SubExpr).hasOperands(const, nonconst) and
-    state = -const.getValue().toInt() and
-    hasSizeImpl(nonconst, n, _)
-  )
-}
+VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
 
 /**
  * Holds if `(n, state)` pair represents the source of flow for the size
  * expression associated with `alloc`.
  */
 predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
-  hasSizeImpl(alloc.getSizeExpr(), n, state)
+  exists(VariableAccess va, Expr size, int delta |
+    size = alloc.getSizeExpr() and
+    // Get the unique variable in a size expression like `x` in `malloc(x + 1)`.
+    va = unique( | | getAVariableAccess(size)) and
+    // Compute `delta` as the constant difference between `x` and `x + 1`.
+    bounded1(any(Instruction instr | instr.getUnconvertedResultExpression() = size),
+      any(LoadInstruction load | load.getUnconvertedResultExpression() = va), delta) and
+    n.asConvertedExpr() = va.getFullyConverted() and
+    state = delta
+  )
 }
 
 /**
@@ -345,6 +324,16 @@ query predicate edges(MergedPathNode node1, MergedPathNode node2) {
   joinOn2(node1.asPathNode3(), node2.asSinkNode(), _)
 }
 
+query predicate subpaths(
+  MergedPathNode arg, MergedPathNode par, MergedPathNode ret, MergedPathNode out
+) {
+  AllocToInvalidPointerFlow::PathGraph1::subpaths(arg.asPathNode1(), par.asPathNode1(),
+    ret.asPathNode1(), out.asPathNode1())
+  or
+  InvalidPointerToDerefFlow::PathGraph::subpaths(arg.asPathNode3(), par.asPathNode3(),
+    ret.asPathNode3(), out.asPathNode3())
+}
+
 /**
  * Holds if `p1` is a sink of `AllocToInvalidPointerConf` and `p2` is a source
  * of `InvalidPointerToDerefConf`, and they are connected through `pai`.

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected

@@ -1,202 +1,62 @@
 edges
-| test.cpp:16:11:16:21 | mk_string_t indirection [string] | test.cpp:24:21:24:31 | call to mk_string_t indirection [string] |
-| test.cpp:16:11:16:21 | mk_string_t indirection [string] | test.cpp:34:21:34:31 | call to mk_string_t indirection [string] |
 | test.cpp:16:11:16:21 | mk_string_t indirection [string] | test.cpp:39:21:39:31 | call to mk_string_t indirection [string] |
 | test.cpp:18:5:18:30 | ... = ... | test.cpp:18:10:18:15 | str indirection [post update] [string] |
 | test.cpp:18:10:18:15 | str indirection [post update] [string] | test.cpp:16:11:16:21 | mk_string_t indirection [string] |
 | test.cpp:18:19:18:24 | call to malloc | test.cpp:18:5:18:30 | ... = ... |
-| test.cpp:24:21:24:31 | call to mk_string_t indirection [string] | test.cpp:26:13:26:15 | str indirection [string] |
-| test.cpp:26:13:26:15 | str indirection [string] | test.cpp:26:18:26:23 | string |
-| test.cpp:26:13:26:15 | str indirection [string] | test.cpp:26:18:26:23 | string indirection |
-| test.cpp:26:18:26:23 | string indirection | test.cpp:26:18:26:23 | string |
-| test.cpp:29:32:29:34 | str indirection [string] | test.cpp:30:13:30:15 | str indirection [string] |
-| test.cpp:30:13:30:15 | str indirection [string] | test.cpp:30:18:30:23 | string |
-| test.cpp:30:13:30:15 | str indirection [string] | test.cpp:30:18:30:23 | string indirection |
-| test.cpp:30:18:30:23 | string indirection | test.cpp:30:18:30:23 | string |
-| test.cpp:34:21:34:31 | call to mk_string_t indirection [string] | test.cpp:35:21:35:23 | str indirection [string] |
-| test.cpp:35:21:35:23 | str indirection [string] | test.cpp:29:32:29:34 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:41:13:41:15 | str indirection [string] |
 | test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:42:13:42:15 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:44:13:44:15 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:45:13:45:15 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:48:17:48:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:52:17:52:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:56:17:56:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:60:17:60:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:64:17:64:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:68:17:68:19 | str indirection [string] |
 | test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:72:17:72:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:76:17:76:19 | str indirection [string] |
 | test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:80:17:80:19 | str indirection [string] |
-| test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | test.cpp:84:17:84:19 | str indirection [string] |
-| test.cpp:41:13:41:15 | str indirection [string] | test.cpp:41:18:41:23 | string |
-| test.cpp:41:13:41:15 | str indirection [string] | test.cpp:41:18:41:23 | string indirection |
-| test.cpp:41:18:41:23 | string indirection | test.cpp:41:18:41:23 | string |
 | test.cpp:42:13:42:15 | str indirection [string] | test.cpp:42:18:42:23 | string |
 | test.cpp:42:13:42:15 | str indirection [string] | test.cpp:42:18:42:23 | string indirection |
 | test.cpp:42:18:42:23 | string indirection | test.cpp:42:18:42:23 | string |
-| test.cpp:44:13:44:15 | str indirection [string] | test.cpp:44:18:44:23 | string |
-| test.cpp:44:13:44:15 | str indirection [string] | test.cpp:44:18:44:23 | string indirection |
-| test.cpp:44:18:44:23 | string indirection | test.cpp:44:18:44:23 | string |
-| test.cpp:45:13:45:15 | str indirection [string] | test.cpp:45:18:45:23 | string |
-| test.cpp:45:13:45:15 | str indirection [string] | test.cpp:45:18:45:23 | string indirection |
-| test.cpp:45:18:45:23 | string indirection | test.cpp:45:18:45:23 | string |
-| test.cpp:48:17:48:19 | str indirection [string] | test.cpp:48:22:48:27 | string |
-| test.cpp:48:17:48:19 | str indirection [string] | test.cpp:48:22:48:27 | string indirection |
-| test.cpp:48:22:48:27 | string indirection | test.cpp:48:22:48:27 | string |
-| test.cpp:52:17:52:19 | str indirection [string] | test.cpp:52:22:52:27 | string |
-| test.cpp:52:17:52:19 | str indirection [string] | test.cpp:52:22:52:27 | string indirection |
-| test.cpp:52:22:52:27 | string indirection | test.cpp:52:22:52:27 | string |
-| test.cpp:56:17:56:19 | str indirection [string] | test.cpp:56:22:56:27 | string |
-| test.cpp:56:17:56:19 | str indirection [string] | test.cpp:56:22:56:27 | string indirection |
-| test.cpp:56:22:56:27 | string indirection | test.cpp:56:22:56:27 | string |
-| test.cpp:60:17:60:19 | str indirection [string] | test.cpp:60:22:60:27 | string |
-| test.cpp:60:17:60:19 | str indirection [string] | test.cpp:60:22:60:27 | string indirection |
-| test.cpp:60:22:60:27 | string indirection | test.cpp:60:22:60:27 | string |
-| test.cpp:64:17:64:19 | str indirection [string] | test.cpp:64:22:64:27 | string |
-| test.cpp:64:17:64:19 | str indirection [string] | test.cpp:64:22:64:27 | string indirection |
-| test.cpp:64:22:64:27 | string indirection | test.cpp:64:22:64:27 | string |
-| test.cpp:68:17:68:19 | str indirection [string] | test.cpp:68:22:68:27 | string |
-| test.cpp:68:17:68:19 | str indirection [string] | test.cpp:68:22:68:27 | string indirection |
-| test.cpp:68:22:68:27 | string indirection | test.cpp:68:22:68:27 | string |
 | test.cpp:72:17:72:19 | str indirection [string] | test.cpp:72:22:72:27 | string |
 | test.cpp:72:17:72:19 | str indirection [string] | test.cpp:72:22:72:27 | string indirection |
 | test.cpp:72:22:72:27 | string indirection | test.cpp:72:22:72:27 | string |
-| test.cpp:76:17:76:19 | str indirection [string] | test.cpp:76:22:76:27 | string |
-| test.cpp:76:17:76:19 | str indirection [string] | test.cpp:76:22:76:27 | string indirection |
-| test.cpp:76:22:76:27 | string indirection | test.cpp:76:22:76:27 | string |
 | test.cpp:80:17:80:19 | str indirection [string] | test.cpp:80:22:80:27 | string |
 | test.cpp:80:17:80:19 | str indirection [string] | test.cpp:80:22:80:27 | string indirection |
 | test.cpp:80:22:80:27 | string indirection | test.cpp:80:22:80:27 | string |
-| test.cpp:84:17:84:19 | str indirection [string] | test.cpp:84:22:84:27 | string |
-| test.cpp:84:17:84:19 | str indirection [string] | test.cpp:84:22:84:27 | string indirection |
-| test.cpp:84:22:84:27 | string indirection | test.cpp:84:22:84:27 | string |
 | test.cpp:88:11:88:30 | mk_string_t_plus_one indirection [string] | test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] |
 | test.cpp:90:5:90:34 | ... = ... | test.cpp:90:10:90:15 | str indirection [post update] [string] |
 | test.cpp:90:10:90:15 | str indirection [post update] [string] | test.cpp:88:11:88:30 | mk_string_t_plus_one indirection [string] |
 | test.cpp:90:19:90:24 | call to malloc | test.cpp:90:5:90:34 | ... = ... |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:98:13:98:15 | str indirection [string] |
 | test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:99:13:99:15 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:101:13:101:15 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:102:13:102:15 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:105:17:105:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:109:17:109:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:113:17:113:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:117:17:117:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:121:17:121:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:125:17:125:19 | str indirection [string] |
 | test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:129:17:129:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:133:17:133:19 | str indirection [string] |
 | test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:137:17:137:19 | str indirection [string] |
-| test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | test.cpp:141:17:141:19 | str indirection [string] |
-| test.cpp:98:13:98:15 | str indirection [string] | test.cpp:98:18:98:23 | string |
-| test.cpp:98:13:98:15 | str indirection [string] | test.cpp:98:18:98:23 | string indirection |
-| test.cpp:98:18:98:23 | string indirection | test.cpp:98:18:98:23 | string |
 | test.cpp:99:13:99:15 | str indirection [string] | test.cpp:99:18:99:23 | string |
 | test.cpp:99:13:99:15 | str indirection [string] | test.cpp:99:18:99:23 | string indirection |
 | test.cpp:99:18:99:23 | string indirection | test.cpp:99:18:99:23 | string |
-| test.cpp:101:13:101:15 | str indirection [string] | test.cpp:101:18:101:23 | string |
-| test.cpp:101:13:101:15 | str indirection [string] | test.cpp:101:18:101:23 | string indirection |
-| test.cpp:101:18:101:23 | string indirection | test.cpp:101:18:101:23 | string |
-| test.cpp:102:13:102:15 | str indirection [string] | test.cpp:102:18:102:23 | string |
-| test.cpp:102:13:102:15 | str indirection [string] | test.cpp:102:18:102:23 | string indirection |
-| test.cpp:102:18:102:23 | string indirection | test.cpp:102:18:102:23 | string |
-| test.cpp:105:17:105:19 | str indirection [string] | test.cpp:105:22:105:27 | string |
-| test.cpp:105:17:105:19 | str indirection [string] | test.cpp:105:22:105:27 | string indirection |
-| test.cpp:105:22:105:27 | string indirection | test.cpp:105:22:105:27 | string |
-| test.cpp:109:17:109:19 | str indirection [string] | test.cpp:109:22:109:27 | string |
-| test.cpp:109:17:109:19 | str indirection [string] | test.cpp:109:22:109:27 | string indirection |
-| test.cpp:109:22:109:27 | string indirection | test.cpp:109:22:109:27 | string |
-| test.cpp:113:17:113:19 | str indirection [string] | test.cpp:113:22:113:27 | string |
-| test.cpp:113:17:113:19 | str indirection [string] | test.cpp:113:22:113:27 | string indirection |
-| test.cpp:113:22:113:27 | string indirection | test.cpp:113:22:113:27 | string |
-| test.cpp:117:17:117:19 | str indirection [string] | test.cpp:117:22:117:27 | string |
-| test.cpp:117:17:117:19 | str indirection [string] | test.cpp:117:22:117:27 | string indirection |
-| test.cpp:117:22:117:27 | string indirection | test.cpp:117:22:117:27 | string |
-| test.cpp:121:17:121:19 | str indirection [string] | test.cpp:121:22:121:27 | string |
-| test.cpp:121:17:121:19 | str indirection [string] | test.cpp:121:22:121:27 | string indirection |
-| test.cpp:121:22:121:27 | string indirection | test.cpp:121:22:121:27 | string |
-| test.cpp:125:17:125:19 | str indirection [string] | test.cpp:125:22:125:27 | string |
-| test.cpp:125:17:125:19 | str indirection [string] | test.cpp:125:22:125:27 | string indirection |
-| test.cpp:125:22:125:27 | string indirection | test.cpp:125:22:125:27 | string |
 | test.cpp:129:17:129:19 | str indirection [string] | test.cpp:129:22:129:27 | string |
 | test.cpp:129:17:129:19 | str indirection [string] | test.cpp:129:22:129:27 | string indirection |
 | test.cpp:129:22:129:27 | string indirection | test.cpp:129:22:129:27 | string |
-| test.cpp:133:17:133:19 | str indirection [string] | test.cpp:133:22:133:27 | string |
-| test.cpp:133:17:133:19 | str indirection [string] | test.cpp:133:22:133:27 | string indirection |
-| test.cpp:133:22:133:27 | string indirection | test.cpp:133:22:133:27 | string |
 | test.cpp:137:17:137:19 | str indirection [string] | test.cpp:137:22:137:27 | string |
 | test.cpp:137:17:137:19 | str indirection [string] | test.cpp:137:22:137:27 | string indirection |
 | test.cpp:137:22:137:27 | string indirection | test.cpp:137:22:137:27 | string |
-| test.cpp:141:17:141:19 | str indirection [string] | test.cpp:141:22:141:27 | string |
-| test.cpp:141:17:141:19 | str indirection [string] | test.cpp:141:22:141:27 | string indirection |
-| test.cpp:141:22:141:27 | string indirection | test.cpp:141:22:141:27 | string |
 | test.cpp:147:5:147:34 | ... = ... | test.cpp:147:10:147:15 | str indirection [post update] [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:150:13:150:15 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:151:13:151:15 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:152:13:152:15 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:154:13:154:15 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:155:13:155:15 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:156:13:156:15 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:159:17:159:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:163:17:163:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:167:17:167:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:171:17:171:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:175:17:175:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:179:17:179:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:183:17:183:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:187:17:187:19 | str indirection [string] |
-| test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:191:17:191:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:195:17:195:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:199:17:199:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:203:17:203:19 | str indirection [string] |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | test.cpp:207:17:207:19 | str indirection [string] |
 | test.cpp:147:19:147:24 | call to malloc | test.cpp:147:5:147:34 | ... = ... |
-| test.cpp:150:13:150:15 | str indirection [string] | test.cpp:150:18:150:23 | string |
-| test.cpp:150:13:150:15 | str indirection [string] | test.cpp:150:18:150:23 | string indirection |
-| test.cpp:150:18:150:23 | string indirection | test.cpp:150:18:150:23 | string |
-| test.cpp:151:13:151:15 | str indirection [string] | test.cpp:151:18:151:23 | string |
-| test.cpp:151:13:151:15 | str indirection [string] | test.cpp:151:18:151:23 | string indirection |
-| test.cpp:151:18:151:23 | string indirection | test.cpp:151:18:151:23 | string |
 | test.cpp:152:13:152:15 | str indirection [string] | test.cpp:152:18:152:23 | string |
 | test.cpp:152:13:152:15 | str indirection [string] | test.cpp:152:18:152:23 | string indirection |
 | test.cpp:152:18:152:23 | string indirection | test.cpp:152:18:152:23 | string |
 | test.cpp:154:13:154:15 | str indirection [string] | test.cpp:154:18:154:23 | string |
 | test.cpp:154:13:154:15 | str indirection [string] | test.cpp:154:18:154:23 | string indirection |
 | test.cpp:154:18:154:23 | string indirection | test.cpp:154:18:154:23 | string |
-| test.cpp:155:13:155:15 | str indirection [string] | test.cpp:155:18:155:23 | string |
-| test.cpp:155:13:155:15 | str indirection [string] | test.cpp:155:18:155:23 | string indirection |
-| test.cpp:155:18:155:23 | string indirection | test.cpp:155:18:155:23 | string |
 | test.cpp:156:13:156:15 | str indirection [string] | test.cpp:156:18:156:23 | string |
 | test.cpp:156:13:156:15 | str indirection [string] | test.cpp:156:18:156:23 | string indirection |
 | test.cpp:156:18:156:23 | string indirection | test.cpp:156:18:156:23 | string |
-| test.cpp:159:17:159:19 | str indirection [string] | test.cpp:159:22:159:27 | string |
-| test.cpp:159:17:159:19 | str indirection [string] | test.cpp:159:22:159:27 | string indirection |
-| test.cpp:159:22:159:27 | string indirection | test.cpp:159:22:159:27 | string |
-| test.cpp:163:17:163:19 | str indirection [string] | test.cpp:163:22:163:27 | string |
-| test.cpp:163:17:163:19 | str indirection [string] | test.cpp:163:22:163:27 | string indirection |
-| test.cpp:163:22:163:27 | string indirection | test.cpp:163:22:163:27 | string |
-| test.cpp:167:17:167:19 | str indirection [string] | test.cpp:167:22:167:27 | string |
-| test.cpp:167:17:167:19 | str indirection [string] | test.cpp:167:22:167:27 | string indirection |
-| test.cpp:167:22:167:27 | string indirection | test.cpp:167:22:167:27 | string |
-| test.cpp:171:17:171:19 | str indirection [string] | test.cpp:171:22:171:27 | string |
-| test.cpp:171:17:171:19 | str indirection [string] | test.cpp:171:22:171:27 | string indirection |
-| test.cpp:171:22:171:27 | string indirection | test.cpp:171:22:171:27 | string |
 | test.cpp:175:17:175:19 | str indirection [string] | test.cpp:175:22:175:27 | string |
 | test.cpp:175:17:175:19 | str indirection [string] | test.cpp:175:22:175:27 | string indirection |
 | test.cpp:175:22:175:27 | string indirection | test.cpp:175:22:175:27 | string |
-| test.cpp:179:17:179:19 | str indirection [string] | test.cpp:179:22:179:27 | string |
-| test.cpp:179:17:179:19 | str indirection [string] | test.cpp:179:22:179:27 | string indirection |
-| test.cpp:179:22:179:27 | string indirection | test.cpp:179:22:179:27 | string |
-| test.cpp:183:17:183:19 | str indirection [string] | test.cpp:183:22:183:27 | string |
-| test.cpp:183:17:183:19 | str indirection [string] | test.cpp:183:22:183:27 | string indirection |
-| test.cpp:183:22:183:27 | string indirection | test.cpp:183:22:183:27 | string |
 | test.cpp:187:17:187:19 | str indirection [string] | test.cpp:187:22:187:27 | string |
 | test.cpp:187:17:187:19 | str indirection [string] | test.cpp:187:22:187:27 | string indirection |
 | test.cpp:187:22:187:27 | string indirection | test.cpp:187:22:187:27 | string |
-| test.cpp:191:17:191:19 | str indirection [string] | test.cpp:191:22:191:27 | string |
-| test.cpp:191:17:191:19 | str indirection [string] | test.cpp:191:22:191:27 | string indirection |
-| test.cpp:191:22:191:27 | string indirection | test.cpp:191:22:191:27 | string |
 | test.cpp:195:17:195:19 | str indirection [string] | test.cpp:195:22:195:27 | string |
 | test.cpp:195:17:195:19 | str indirection [string] | test.cpp:195:22:195:27 | string indirection |
 | test.cpp:195:22:195:27 | string indirection | test.cpp:195:22:195:27 | string |
@@ -222,159 +82,57 @@ edges
 | test.cpp:243:12:243:14 | str indirection [string] | test.cpp:243:12:243:21 | string |
 | test.cpp:243:12:243:14 | str indirection [string] | test.cpp:243:16:243:21 | string indirection |
 | test.cpp:243:16:243:21 | string indirection | test.cpp:243:12:243:21 | string |
+| test.cpp:249:20:249:27 | call to my_alloc | test.cpp:250:12:250:12 | p |
+| test.cpp:256:17:256:22 | call to malloc | test.cpp:257:12:257:12 | p |
+| test.cpp:262:22:262:27 | call to malloc | test.cpp:266:12:266:12 | p |
+| test.cpp:264:20:264:25 | call to malloc | test.cpp:266:12:266:12 | p |
 nodes
 | test.cpp:16:11:16:21 | mk_string_t indirection [string] | semmle.label | mk_string_t indirection [string] |
 | test.cpp:18:5:18:30 | ... = ... | semmle.label | ... = ... |
 | test.cpp:18:10:18:15 | str indirection [post update] [string] | semmle.label | str indirection [post update] [string] |
 | test.cpp:18:19:18:24 | call to malloc | semmle.label | call to malloc |
-| test.cpp:24:21:24:31 | call to mk_string_t indirection [string] | semmle.label | call to mk_string_t indirection [string] |
-| test.cpp:26:13:26:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:26:18:26:23 | string | semmle.label | string |
-| test.cpp:26:18:26:23 | string indirection | semmle.label | string indirection |
-| test.cpp:29:32:29:34 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:30:13:30:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:30:18:30:23 | string | semmle.label | string |
-| test.cpp:30:18:30:23 | string indirection | semmle.label | string indirection |
-| test.cpp:34:21:34:31 | call to mk_string_t indirection [string] | semmle.label | call to mk_string_t indirection [string] |
-| test.cpp:35:21:35:23 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:39:21:39:31 | call to mk_string_t indirection [string] | semmle.label | call to mk_string_t indirection [string] |
-| test.cpp:41:13:41:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:41:18:41:23 | string | semmle.label | string |
-| test.cpp:41:18:41:23 | string indirection | semmle.label | string indirection |
 | test.cpp:42:13:42:15 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:42:18:42:23 | string | semmle.label | string |
 | test.cpp:42:18:42:23 | string indirection | semmle.label | string indirection |
-| test.cpp:44:13:44:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:44:18:44:23 | string | semmle.label | string |
-| test.cpp:44:18:44:23 | string indirection | semmle.label | string indirection |
-| test.cpp:45:13:45:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:45:18:45:23 | string | semmle.label | string |
-| test.cpp:45:18:45:23 | string indirection | semmle.label | string indirection |
-| test.cpp:48:17:48:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:48:22:48:27 | string | semmle.label | string |
-| test.cpp:48:22:48:27 | string indirection | semmle.label | string indirection |
-| test.cpp:52:17:52:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:52:22:52:27 | string | semmle.label | string |
-| test.cpp:52:22:52:27 | string indirection | semmle.label | string indirection |
-| test.cpp:56:17:56:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:56:22:56:27 | string | semmle.label | string |
-| test.cpp:56:22:56:27 | string indirection | semmle.label | string indirection |
-| test.cpp:60:17:60:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:60:22:60:27 | string | semmle.label | string |
-| test.cpp:60:22:60:27 | string indirection | semmle.label | string indirection |
-| test.cpp:64:17:64:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:64:22:64:27 | string | semmle.label | string |
-| test.cpp:64:22:64:27 | string indirection | semmle.label | string indirection |
-| test.cpp:68:17:68:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:68:22:68:27 | string | semmle.label | string |
-| test.cpp:68:22:68:27 | string indirection | semmle.label | string indirection |
 | test.cpp:72:17:72:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:72:22:72:27 | string | semmle.label | string |
 | test.cpp:72:22:72:27 | string indirection | semmle.label | string indirection |
-| test.cpp:76:17:76:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:76:22:76:27 | string | semmle.label | string |
-| test.cpp:76:22:76:27 | string indirection | semmle.label | string indirection |
 | test.cpp:80:17:80:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:80:22:80:27 | string | semmle.label | string |
 | test.cpp:80:22:80:27 | string indirection | semmle.label | string indirection |
-| test.cpp:84:17:84:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:84:22:84:27 | string | semmle.label | string |
-| test.cpp:84:22:84:27 | string indirection | semmle.label | string indirection |
 | test.cpp:88:11:88:30 | mk_string_t_plus_one indirection [string] | semmle.label | mk_string_t_plus_one indirection [string] |
 | test.cpp:90:5:90:34 | ... = ... | semmle.label | ... = ... |
 | test.cpp:90:10:90:15 | str indirection [post update] [string] | semmle.label | str indirection [post update] [string] |
 | test.cpp:90:19:90:24 | call to malloc | semmle.label | call to malloc |
 | test.cpp:96:21:96:40 | call to mk_string_t_plus_one indirection [string] | semmle.label | call to mk_string_t_plus_one indirection [string] |
-| test.cpp:98:13:98:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:98:18:98:23 | string | semmle.label | string |
-| test.cpp:98:18:98:23 | string indirection | semmle.label | string indirection |
 | test.cpp:99:13:99:15 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:99:18:99:23 | string | semmle.label | string |
 | test.cpp:99:18:99:23 | string indirection | semmle.label | string indirection |
-| test.cpp:101:13:101:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:101:18:101:23 | string | semmle.label | string |
-| test.cpp:101:18:101:23 | string indirection | semmle.label | string indirection |
-| test.cpp:102:13:102:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:102:18:102:23 | string | semmle.label | string |
-| test.cpp:102:18:102:23 | string indirection | semmle.label | string indirection |
-| test.cpp:105:17:105:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:105:22:105:27 | string | semmle.label | string |
-| test.cpp:105:22:105:27 | string indirection | semmle.label | string indirection |
-| test.cpp:109:17:109:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:109:22:109:27 | string | semmle.label | string |
-| test.cpp:109:22:109:27 | string indirection | semmle.label | string indirection |
-| test.cpp:113:17:113:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:113:22:113:27 | string | semmle.label | string |
-| test.cpp:113:22:113:27 | string indirection | semmle.label | string indirection |
-| test.cpp:117:17:117:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:117:22:117:27 | string | semmle.label | string |
-| test.cpp:117:22:117:27 | string indirection | semmle.label | string indirection |
-| test.cpp:121:17:121:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:121:22:121:27 | string | semmle.label | string |
-| test.cpp:121:22:121:27 | string indirection | semmle.label | string indirection |
-| test.cpp:125:17:125:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:125:22:125:27 | string | semmle.label | string |
-| test.cpp:125:22:125:27 | string indirection | semmle.label | string indirection |
 | test.cpp:129:17:129:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:129:22:129:27 | string | semmle.label | string |
 | test.cpp:129:22:129:27 | string indirection | semmle.label | string indirection |
-| test.cpp:133:17:133:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:133:22:133:27 | string | semmle.label | string |
-| test.cpp:133:22:133:27 | string indirection | semmle.label | string indirection |
 | test.cpp:137:17:137:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:137:22:137:27 | string | semmle.label | string |
 | test.cpp:137:22:137:27 | string indirection | semmle.label | string indirection |
-| test.cpp:141:17:141:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:141:22:141:27 | string | semmle.label | string |
-| test.cpp:141:22:141:27 | string indirection | semmle.label | string indirection |
 | test.cpp:147:5:147:34 | ... = ... | semmle.label | ... = ... |
 | test.cpp:147:10:147:15 | str indirection [post update] [string] | semmle.label | str indirection [post update] [string] |
 | test.cpp:147:19:147:24 | call to malloc | semmle.label | call to malloc |
-| test.cpp:150:13:150:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:150:18:150:23 | string | semmle.label | string |
-| test.cpp:150:18:150:23 | string indirection | semmle.label | string indirection |
-| test.cpp:151:13:151:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:151:18:151:23 | string | semmle.label | string |
-| test.cpp:151:18:151:23 | string indirection | semmle.label | string indirection |
 | test.cpp:152:13:152:15 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:152:18:152:23 | string | semmle.label | string |
 | test.cpp:152:18:152:23 | string indirection | semmle.label | string indirection |
 | test.cpp:154:13:154:15 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:154:18:154:23 | string | semmle.label | string |
 | test.cpp:154:18:154:23 | string indirection | semmle.label | string indirection |
-| test.cpp:155:13:155:15 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:155:18:155:23 | string | semmle.label | string |
-| test.cpp:155:18:155:23 | string indirection | semmle.label | string indirection |
 | test.cpp:156:13:156:15 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:156:18:156:23 | string | semmle.label | string |
 | test.cpp:156:18:156:23 | string indirection | semmle.label | string indirection |
-| test.cpp:159:17:159:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:159:22:159:27 | string | semmle.label | string |
-| test.cpp:159:22:159:27 | string indirection | semmle.label | string indirection |
-| test.cpp:163:17:163:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:163:22:163:27 | string | semmle.label | string |
-| test.cpp:163:22:163:27 | string indirection | semmle.label | string indirection |
-| test.cpp:167:17:167:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:167:22:167:27 | string | semmle.label | string |
-| test.cpp:167:22:167:27 | string indirection | semmle.label | string indirection |
-| test.cpp:171:17:171:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:171:22:171:27 | string | semmle.label | string |
-| test.cpp:171:22:171:27 | string indirection | semmle.label | string indirection |
 | test.cpp:175:17:175:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:175:22:175:27 | string | semmle.label | string |
 | test.cpp:175:22:175:27 | string indirection | semmle.label | string indirection |
-| test.cpp:179:17:179:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:179:22:179:27 | string | semmle.label | string |
-| test.cpp:179:22:179:27 | string indirection | semmle.label | string indirection |
-| test.cpp:183:17:183:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:183:22:183:27 | string | semmle.label | string |
-| test.cpp:183:22:183:27 | string indirection | semmle.label | string indirection |
 | test.cpp:187:17:187:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:187:22:187:27 | string | semmle.label | string |
 | test.cpp:187:22:187:27 | string indirection | semmle.label | string indirection |
-| test.cpp:191:17:191:19 | str indirection [string] | semmle.label | str indirection [string] |
-| test.cpp:191:22:191:27 | string | semmle.label | string |
-| test.cpp:191:22:191:27 | string indirection | semmle.label | string indirection |
 | test.cpp:195:17:195:19 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:195:22:195:27 | string | semmle.label | string |
 | test.cpp:195:22:195:27 | string indirection | semmle.label | string indirection |
@@ -402,6 +160,13 @@ nodes
 | test.cpp:243:12:243:14 | str indirection [string] | semmle.label | str indirection [string] |
 | test.cpp:243:12:243:21 | string | semmle.label | string |
 | test.cpp:243:16:243:21 | string indirection | semmle.label | string indirection |
+| test.cpp:249:20:249:27 | call to my_alloc | semmle.label | call to my_alloc |
+| test.cpp:250:12:250:12 | p | semmle.label | p |
+| test.cpp:256:17:256:22 | call to malloc | semmle.label | call to malloc |
+| test.cpp:257:12:257:12 | p | semmle.label | p |
+| test.cpp:262:22:262:27 | call to malloc | semmle.label | call to malloc |
+| test.cpp:264:20:264:25 | call to malloc | semmle.label | call to malloc |
+| test.cpp:266:12:266:12 | p | semmle.label | p |
 subpaths
 | test.cpp:242:22:242:27 | buffer | test.cpp:235:40:235:45 | buffer | test.cpp:236:12:236:17 | p_str indirection [post update] [string] | test.cpp:242:16:242:19 | set_string output argument [string] |
 #select
@@ -420,5 +185,6 @@ subpaths
 | test.cpp:199:9:199:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:199:22:199:27 | string | This write may overflow $@ by 2 elements. | test.cpp:199:22:199:27 | string | string |
 | test.cpp:203:9:203:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:203:22:203:27 | string | This write may overflow $@ by 2 elements. | test.cpp:203:22:203:27 | string | string |
 | test.cpp:207:9:207:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:207:22:207:27 | string | This write may overflow $@ by 3 elements. | test.cpp:207:22:207:27 | string | string |
-| test.cpp:232:3:232:8 | call to memset | test.cpp:228:43:228:48 | call to malloc | test.cpp:232:10:232:15 | buffer | This write may overflow $@ by 32 elements. | test.cpp:232:10:232:15 | buffer | buffer |
 | test.cpp:243:5:243:10 | call to memset | test.cpp:241:27:241:32 | call to malloc | test.cpp:243:12:243:21 | string | This write may overflow $@ by 1 element. | test.cpp:243:16:243:21 | string | string |
+| test.cpp:250:5:250:10 | call to memset | test.cpp:249:20:249:27 | call to my_alloc | test.cpp:250:12:250:12 | p | This write may overflow $@ by 1 element. | test.cpp:250:12:250:12 | p | p |
+| test.cpp:266:5:266:10 | call to memset | test.cpp:262:22:262:27 | call to malloc | test.cpp:266:12:266:12 | p | This write may overflow $@ by 1 element. | test.cpp:266:12:266:12 | p | p |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp

@@ -213,7 +213,7 @@ void *memset(void *, int, unsigned);
 
 void call_memset(void *p, unsigned size)
 {
-  memset(p, 0, size); // GOOD [FALSE POSITIVE]
+  memset(p, 0, size); // GOOD
 }
 
 void test_missing_call_context(unsigned char *unrelated_buffer, unsigned size) {
@@ -229,7 +229,7 @@ void repeated_alerts(unsigned size, unsigned offset) {
   while(unknown()) {
     ++size;
   }
-  memset(buffer, 0, size); // BAD
+  memset(buffer, 0, size); // BAD [NOT DETECTED]
 }
 
 void set_string(string_t* p_str, char* buffer) {
@@ -243,3 +243,25 @@ void test_flow_through_setter(unsigned size) {
     memset(str.string, 0, size + 1); // BAD
 }
 
+void* my_alloc(unsigned size);
+
+void foo(unsigned size) {
+    int* p = (int*)my_alloc(size); // BAD
+    memset(p, 0, size + 1);
+}
+
+void test6(unsigned long n, char *p) {
+  while (unknown()) {
+    n++;
+    p = (char *)malloc(n);
+    memset(p, 0, n); // GOOD
+  }
+}
+
+void test7(unsigned n) {
+    char* p = (char*)malloc(n);
+    if(!p) {
+        p = (char*)malloc(++n);
+    }
+    memset(p, 0, n); // GOOD [FALSE POSITIVE]
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected

@@ -649,6 +649,36 @@ edges
 | test.cpp:280:13:280:24 | new[] | test.cpp:281:14:281:15 | xs |
 | test.cpp:290:13:290:24 | new[] | test.cpp:291:14:291:15 | xs |
 | test.cpp:290:13:290:24 | new[] | test.cpp:292:30:292:30 | x |
+| test.cpp:304:15:304:26 | new[] | test.cpp:307:5:307:6 | xs |
+| test.cpp:304:15:304:26 | new[] | test.cpp:308:5:308:6 | xs |
+| test.cpp:308:5:308:6 | xs | test.cpp:308:5:308:11 | access to array |
+| test.cpp:308:5:308:11 | access to array | test.cpp:308:5:308:29 | Store: ... = ... |
+| test.cpp:313:16:313:29 | new[] | test.cpp:314:17:314:18 | xs |
+| test.cpp:314:17:314:18 | xs | test.cpp:314:17:314:25 | ... + ... |
+| test.cpp:314:17:314:18 | xs | test.cpp:314:17:314:25 | ... + ... |
+| test.cpp:314:17:314:18 | xs | test.cpp:318:13:318:20 | * ... |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:318:14:318:20 | current |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:318:14:318:20 | current |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:13:320:20 | * ... |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:13:320:20 | * ... |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:14:320:20 | current |
+| test.cpp:314:17:314:25 | ... + ... | test.cpp:320:14:320:20 | current |
+| test.cpp:318:13:318:20 | * ... | test.cpp:318:14:318:20 | current |
+| test.cpp:318:13:318:20 | * ... | test.cpp:320:13:320:20 | * ... |
+| test.cpp:318:13:318:20 | * ... | test.cpp:320:14:320:20 | current |
+| test.cpp:318:14:318:20 | current | test.cpp:314:17:314:25 | Store: ... + ... |
+| test.cpp:318:14:318:20 | current | test.cpp:318:13:318:20 | Load: * ... |
+| test.cpp:318:14:318:20 | current | test.cpp:320:10:320:21 | Store: -- ... |
+| test.cpp:318:14:318:20 | current | test.cpp:320:12:320:21 | Load: (...) |
+| test.cpp:320:13:320:20 | * ... | test.cpp:314:17:314:25 | Store: ... + ... |
+| test.cpp:320:13:320:20 | * ... | test.cpp:318:13:318:20 | Load: * ... |
+| test.cpp:320:13:320:20 | * ... | test.cpp:320:10:320:21 | Store: -- ... |
+| test.cpp:320:13:320:20 | * ... | test.cpp:320:12:320:21 | Load: (...) |
+| test.cpp:320:14:320:20 | current | test.cpp:314:17:314:25 | Store: ... + ... |
+| test.cpp:320:14:320:20 | current | test.cpp:318:13:318:20 | Load: * ... |
+| test.cpp:320:14:320:20 | current | test.cpp:320:10:320:21 | Store: -- ... |
+| test.cpp:320:14:320:20 | current | test.cpp:320:12:320:21 | Load: (...) |
+subpaths
 #select
 | test.cpp:6:14:6:15 | Load: * ... | test.cpp:4:15:4:20 | call to malloc | test.cpp:6:14:6:15 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:4:15:4:20 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size |
 | test.cpp:8:14:8:21 | Load: * ... | test.cpp:4:15:4:20 | call to malloc | test.cpp:8:14:8:21 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:4:15:4:20 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size |
@@ -672,3 +702,8 @@ edges
 | test.cpp:254:9:254:16 | Store: ... = ... | test.cpp:248:24:248:30 | call to realloc | test.cpp:254:9:254:16 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:248:24:248:30 | call to realloc | call to realloc | test.cpp:254:11:254:11 | i | i |
 | test.cpp:264:13:264:14 | Load: * ... | test.cpp:260:13:260:24 | new[] | test.cpp:264:13:264:14 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:260:13:260:24 | new[] | new[] | test.cpp:261:19:261:21 | len | len |
 | test.cpp:274:5:274:10 | Store: ... = ... | test.cpp:270:13:270:24 | new[] | test.cpp:274:5:274:10 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:270:13:270:24 | new[] | new[] | test.cpp:271:19:271:21 | len | len |
+| test.cpp:308:5:308:29 | Store: ... = ... | test.cpp:304:15:304:26 | new[] | test.cpp:308:5:308:29 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:304:15:304:26 | new[] | new[] | test.cpp:308:8:308:10 | ... + ... | ... + ... |
+| test.cpp:314:17:314:25 | Store: ... + ... | test.cpp:313:16:313:29 | new[] | test.cpp:314:17:314:25 | Store: ... + ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
+| test.cpp:318:13:318:20 | Load: * ... | test.cpp:313:16:313:29 | new[] | test.cpp:318:13:318:20 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
+| test.cpp:320:10:320:21 | Store: -- ... | test.cpp:313:16:313:29 | new[] | test.cpp:320:10:320:21 | Store: -- ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |
+| test.cpp:320:12:320:21 | Load: (...) | test.cpp:313:16:313:29 | new[] | test.cpp:320:12:320:21 | Load: (...) | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:313:16:313:29 | new[] | new[] | test.cpp:314:22:314:25 | size | size |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp

@@ -293,4 +293,32 @@ void test20(unsigned len)
   {
     *x = 0; // GOOD
   }
-}
+}
+
+void* test21_get(int n);
+
+void test21() {
+  int n = 0;
+  while (test21_get(n)) n+=2;
+
+  void** xs = new void*[n];
+
+  for (int i = 0; i < n; i += 2) {
+    xs[i] = test21_get(i); // GOOD
+    xs[i+1] = test21_get(i+1); // GOOD [FALSE POSITIVE]
+  }
+}
+
+void test22(unsigned size, int val) {
+    char *xs = new char[size];
+    char *end = xs + size; // GOOD [FALSE POSITIVE]
+    char **current = &end;
+    do
+    {
+        if( *current - xs < 1 ) // GOOD [FALSE POSITIVE]
+            return;
+        *--(*current) = 0; // GOOD [FALSE POSITIVE]
+        val >>= 8;
+    }
+    while( val > 0 );
+}

cpp/ql/test/library-tests/dataflow/dataflow-tests/clang.cpp

@@ -1,7 +1,7 @@
 // semmle-extractor-options: --edg --clang
 
 int source();
-void sink(int); void sink(const int *); void sink(int **);
+void sink(int); void sink(const int *); void sink(int **); void indirect_sink(...);
 
 struct twoIntFields {
   int m1, m2;
@@ -19,7 +19,8 @@ void following_pointers( // $ ast-def=sourceStruct1_ptr
 
   sink(sourceArray1[0]); // no flow
   sink(*sourceArray1); // no flow
-  sink(&sourceArray1); // $ ast,ir // [should probably be taint only]
+  sink(&sourceArray1); // $ ast // [should probably be taint only]
+  indirect_sink(&sourceArray1); // $ ast,ir
 
   sink(sourceStruct1.m1); // no flow
   sink(sourceStruct1_ptr->m1); // no flow
@@ -48,5 +49,6 @@ void following_pointers( // $ ast-def=sourceStruct1_ptr
 
   int stackArray[2] = { source(), source() };
   stackArray[0] = source();
-  sink(stackArray); // $ ast ir ir=49:25 ir=49:35 ir=50:19
+  sink(stackArray); // $ ast,ir
+  indirect_sink(stackArray); // $ ast ir=50:25 ir=50:35 ir=51:19
 }

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected

@@ -28,9 +28,10 @@ postWithInFlow
 | BarrierGuard.cpp:49:6:49:6 | x [post update] | PostUpdateNode should not be the target of local flow. |
 | BarrierGuard.cpp:60:7:60:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
 | clang.cpp:22:9:22:20 | sourceArray1 [inner post update] | PostUpdateNode should not be the target of local flow. |
-| clang.cpp:28:22:28:23 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
-| clang.cpp:50:3:50:12 | stackArray [inner post update] | PostUpdateNode should not be the target of local flow. |
-| clang.cpp:50:3:50:15 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
+| clang.cpp:23:18:23:29 | sourceArray1 [inner post update] | PostUpdateNode should not be the target of local flow. |
+| clang.cpp:29:22:29:23 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
+| clang.cpp:51:3:51:12 | stackArray [inner post update] | PostUpdateNode should not be the target of local flow. |
+| clang.cpp:51:3:51:15 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
 | dispatch.cpp:60:3:60:14 | globalBottom [post update] | PostUpdateNode should not be the target of local flow. |
 | dispatch.cpp:61:3:61:14 | globalMiddle [post update] | PostUpdateNode should not be the target of local flow. |
 | dispatch.cpp:78:24:78:37 | call to allocateBottom [inner post update] | PostUpdateNode should not be the target of local flow. |
@@ -125,6 +126,8 @@ postWithInFlow
 | test.cpp:681:3:681:3 | s [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:689:3:689:3 | s [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:690:3:690:3 | s [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:694:4:694:6 | buf [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:704:23:704:25 | buf [inner post update] | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
 uniqueParameterNodePosition

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

@@ -1,5 +1,5 @@
 int source();
-void sink(int); void sink(const int *); void sink(int **);
+void sink(int); void sink(const int *); void sink(int **); void indirect_sink(...);
 
 void intraprocedural_with_local_flow() {
   int t2;
@@ -626,7 +626,7 @@ void test_def_via_phi_read(bool b)
     use(buffer);
   }
   intPointerSource(buffer);
-  sink(buffer); // $ ast,ir
+  indirect_sink(buffer); // $ ast,ir
 }
 
 void test_static_local_1() {
@@ -690,3 +690,16 @@ void test_static_local_9() {
   s = 0;
 }
 
+void increment_buf(int** buf) { // $ ast-def=buf ir-def=*buf ir-def=**buf
+  *buf += 10;
+  sink(buf); // $ SPURIOUS: ast
+}
+
+void call_increment_buf(int** buf) { // $ ast-def=buf
+  increment_buf(buf);
+}
+
+void test_conflation_regression(int* source) { // $ ast-def=source
+  int* buf = source;
+  call_increment_buf(&buf);
+}

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

@@ -34,7 +34,7 @@ module AstTest {
 
     override predicate isSink(DataFlow::Node sink) {
       exists(FunctionCall call |
-        call.getTarget().getName() = "sink" and
+        call.getTarget().getName() = ["sink", "indirect_sink"] and
         sink.asExpr() = call.getAnArgument()
       )
     }
@@ -83,9 +83,12 @@ module IRTest {
     }
 
     override predicate isSink(DataFlow::Node sink) {
-      exists(FunctionCall call |
+      exists(FunctionCall call, Expr e | e = call.getAnArgument() |
         call.getTarget().getName() = "sink" and
-        call.getAnArgument() in [sink.asExpr(), sink.asIndirectExpr()]
+        sink.asExpr() = e
+        or
+        call.getTarget().getName() = "indirect_sink" and
+        sink.asIndirectExpr() = e
       )
     }
 

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

@@ -49,3 +49,13 @@
     return 0;
   }
 
+  void* f3_get(int n);
+
+  void f3() {
+    int n = 0;
+    while (f3_get(n)) n+=2;
+
+    for (int i = 0; i < n; i += 2) {
+      range(i); // $ range=>=0 SPURIOUS: range="<=call to f3_get-1" range="<=call to f3_get-2"
+    }
+  }

cpp/ql/test/query-tests/Security/CWE/CWE-078/SAMATE/ExecTainted/ExecTainted.expected

@@ -1,29 +1,16 @@
 edges
 | tests.cpp:26:15:26:23 | badSource indirection | tests.cpp:51:12:51:20 | call to badSource indirection |
-| tests.cpp:26:32:26:35 | data indirection | tests.cpp:26:15:26:23 | badSource indirection |
-| tests.cpp:26:32:26:35 | data indirection | tests.cpp:38:25:38:36 | strncat output argument |
 | tests.cpp:33:34:33:39 | call to getenv indirection | tests.cpp:38:39:38:49 | environment indirection |
 | tests.cpp:38:25:38:36 | strncat output argument | tests.cpp:26:15:26:23 | badSource indirection |
-| tests.cpp:38:25:38:36 | strncat output argument | tests.cpp:26:15:26:23 | badSource indirection |
-| tests.cpp:38:25:38:36 | strncat output argument | tests.cpp:51:22:51:25 | badSource output argument |
 | tests.cpp:38:39:38:49 | environment indirection | tests.cpp:38:25:38:36 | strncat output argument |
 | tests.cpp:51:12:51:20 | call to badSource indirection | tests.cpp:53:16:53:19 | data indirection |
-| tests.cpp:51:22:51:25 | badSource output argument | tests.cpp:51:22:51:25 | data indirection |
-| tests.cpp:51:22:51:25 | data indirection | tests.cpp:26:32:26:35 | data indirection |
-| tests.cpp:51:22:51:25 | data indirection | tests.cpp:51:12:51:20 | call to badSource indirection |
 nodes
 | tests.cpp:26:15:26:23 | badSource indirection | semmle.label | badSource indirection |
-| tests.cpp:26:15:26:23 | badSource indirection | semmle.label | badSource indirection |
-| tests.cpp:26:32:26:35 | data indirection | semmle.label | data indirection |
 | tests.cpp:33:34:33:39 | call to getenv indirection | semmle.label | call to getenv indirection |
 | tests.cpp:38:25:38:36 | strncat output argument | semmle.label | strncat output argument |
-| tests.cpp:38:25:38:36 | strncat output argument | semmle.label | strncat output argument |
 | tests.cpp:38:39:38:49 | environment indirection | semmle.label | environment indirection |
 | tests.cpp:51:12:51:20 | call to badSource indirection | semmle.label | call to badSource indirection |
-| tests.cpp:51:22:51:25 | badSource output argument | semmle.label | badSource output argument |
-| tests.cpp:51:22:51:25 | data indirection | semmle.label | data indirection |
 | tests.cpp:53:16:53:19 | data indirection | semmle.label | data indirection |
 subpaths
-| tests.cpp:51:22:51:25 | data indirection | tests.cpp:26:32:26:35 | data indirection | tests.cpp:26:15:26:23 | badSource indirection | tests.cpp:51:12:51:20 | call to badSource indirection |
 #select
 | tests.cpp:53:16:53:19 | data | tests.cpp:33:34:33:39 | call to getenv indirection | tests.cpp:53:16:53:19 | data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | tests.cpp:33:34:33:39 | call to getenv indirection | user input (an environment variable) | tests.cpp:38:25:38:36 | strncat output argument | strncat output argument |

cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected

@@ -45,8 +45,6 @@ edges
 | test.cpp:186:47:186:54 | filename indirection | test.cpp:188:20:188:24 | flags indirection |
 | test.cpp:187:11:187:15 | strncat output argument | test.cpp:188:11:188:17 | strncat output argument |
 | test.cpp:187:18:187:25 | filename indirection | test.cpp:187:11:187:15 | strncat output argument |
-| test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | strncat output argument |
-| test.cpp:188:11:188:17 | strncat output argument | test.cpp:188:11:188:17 | strncat output argument |
 | test.cpp:188:20:188:24 | flags indirection | test.cpp:188:11:188:17 | strncat output argument |
 | test.cpp:194:9:194:16 | fread output argument | test.cpp:196:26:196:33 | filename indirection |
 | test.cpp:196:10:196:16 | concat output argument | test.cpp:198:32:198:38 | command indirection |

csharp/ql/consistency-queries/DataFlowConsistency.ql

@@ -72,5 +72,5 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
 
   override predicate reverseReadExclude(Node n) { n.asExpr() = any(AwaitExpr ae).getExpr() }
 
-  override predicate identityLocalStepExclude(Node n) { n.getLocation().getFile().fromLibrary() }
+  override predicate identityLocalStepExclude(Node n) { none() }
 }

csharp/ql/lib/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Minor Analysis Improvements
 
+* The `cs/log-forging`, `cs/cleartext-storage`, and `cs/exposure-of-sensitive-information` queries now correctly handle unsanitized arguments to `ILogger` extension methods.
 * Updated the `neutralModel` extensible predicate to include a `kind` column.
 
 ## 0.6.1

csharp/ql/lib/change-notes/released/0.6.2.md

@@ -2,4 +2,5 @@
 
 ### Minor Analysis Improvements
 
+* The `cs/log-forging`, `cs/cleartext-storage`, and `cs/exposure-of-sensitive-information` queries now correctly handle unsanitized arguments to `ILogger` extension methods.
 * Updated the `neutralModel` extensible predicate to include a `kind` column.

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -335,7 +335,8 @@ module LocalFlow {
     exists(ControlFlow::BasicBlock bb, int i |
       SsaImpl::lastRefBeforeRedefExt(def, bb, i, next.getDefinitionExt()) and
       def.definesAt(_, bb, i, _) and
-      def = getSsaDefinitionExt(nodeFrom)
+      def = getSsaDefinitionExt(nodeFrom) and
+      nodeFrom != next
     )
   }
 
@@ -414,7 +415,8 @@ module LocalFlow {
     ) {
       exists(CIL::BasicBlock bb, int i | CilSsaImpl::lastRefBeforeRedefExt(def, bb, i, next) |
         def.definesAt(_, bb, i, _) and
-        def = nodeFrom.(CilSsaDefinitionExtNode).getDefinition()
+        def = nodeFrom.(CilSsaDefinitionExtNode).getDefinition() and
+        def != next
         or
         nodeFrom = TCilExprNode(bb.getNode(i).(CIL::ReadAccess))
       )
@@ -440,7 +442,8 @@ module LocalFlow {
       exists(CIL::ReadAccess readFrom, CIL::ReadAccess readTo |
         CilSsaImpl::hasAdjacentReadsExt(def, readFrom, readTo) and
         nodeTo = TCilExprNode(readTo) and
-        nodeFrom = TCilExprNode(readFrom)
+        nodeFrom = TCilExprNode(readFrom) and
+        nodeFrom != nodeTo
       )
       or
       // Flow into phi (read) node
@@ -483,7 +486,8 @@ module LocalFlow {
     or
     hasNodePath(any(LocalExprStepConfiguration x), nodeFrom, nodeTo)
     or
-    ThisFlow::adjacentThisRefs(nodeFrom, nodeTo)
+    ThisFlow::adjacentThisRefs(nodeFrom, nodeTo) and
+    nodeFrom != nodeTo
     or
     ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
     or
@@ -541,7 +545,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   exists(SsaImpl::DefinitionExt def |
     LocalFlow::localSsaFlowStepUseUse(def, nodeFrom, nodeTo) and
     not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _) and
-    not LocalFlow::usesInstanceField(def)
+    not LocalFlow::usesInstanceField(def) and
+    nodeFrom != nodeTo
   )
   or
   // Flow into phi (read)/uncertain SSA definition node from read
@@ -880,7 +885,8 @@ private module Cached {
   predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
     or
-    LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
+    LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo) and
+    nodeFrom != nodeTo
     or
     exists(SsaImpl::DefinitionExt def |
       LocalFlow::localSsaFlowStep(def, nodeFrom, nodeTo) and

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll

@@ -26,7 +26,14 @@ private class ExternalModelSink extends ExternalLocationSink {
  * An argument to a call to a method on a logger class.
  */
 class LogMessageSink extends ExternalLocationSink {
-  LogMessageSink() { this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument() }
+  LogMessageSink() {
+    this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument()
+    or
+    this.getExpr() =
+      any(ExtensionMethodCall call |
+        call.getTarget().(ExtensionMethod).getExtendedType() instanceof LoggerType
+      ).getArgument(any(int i | i > 0))
+  }
 }
 
 /**

csharp/ql/test/experimental/ir/offbyone/CONSISTENCY/DataFlowConsistency.expected

@@ -1,7 +0,0 @@
-identityLocalStep
-| test.cs:17:41:17:44 | this access | Node steps to itself |
-| test.cs:34:41:34:44 | this access | Node steps to itself |
-| test.cs:52:41:52:44 | this access | Node steps to itself |
-| test.cs:67:41:67:44 | this access | Node steps to itself |
-| test.cs:77:22:77:24 | this access | Node steps to itself |
-| test.cs:90:41:90:44 | this access | Node steps to itself |

csharp/ql/test/library-tests/controlflow/graph/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +0,0 @@
-identityLocalStep
-| Conditions.cs:133:17:133:22 | [Field1 (line 129): false] this access | Node steps to itself |

csharp/ql/test/library-tests/controlflow/guards/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +0,0 @@
-identityLocalStep
-| Splitting.cs:133:21:133:29 | [b (line 123): false] this access | Node steps to itself |

csharp/ql/test/library-tests/controlflow/splits/CONSISTENCY/DataFlowConsistency.expected

@@ -1,7 +0,0 @@
-identityLocalStep
-| SplittingStressTest.cs:172:16:172:16 | SSA phi read(b29) | Node steps to itself |
-| SplittingStressTest.cs:179:13:183:13 | [b1 (line 170): false] SSA phi read(b1) | Node steps to itself |
-| SplittingStressTest.cs:184:13:188:13 | [b2 (line 170): false] SSA phi read(b2) | Node steps to itself |
-| SplittingStressTest.cs:189:13:193:13 | [b3 (line 170): false] SSA phi read(b3) | Node steps to itself |
-| SplittingStressTest.cs:194:13:198:13 | [b4 (line 170): false] SSA phi read(b4) | Node steps to itself |
-| SplittingStressTest.cs:199:13:203:13 | [b5 (line 170): false] SSA phi read(b5) | Node steps to itself |

csharp/ql/test/library-tests/dataflow/defuse/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +0,0 @@
-identityLocalStep
-| Test.cs:80:37:80:42 | this access | Node steps to itself |

csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs

@@ -214,4 +214,25 @@ namespace My.Qltest
 
         static void Sink(object o) { }
     }
+
+    public interface HI { }
+
+    public class HC : HI { }
+
+    public static class HE
+    {
+        public static object ExtensionMethod(this HI h) => throw null;
+    }
+
+    public class H
+    {
+        void M1()
+        {
+            var h = new HC();
+            var o = h.ExtensionMethod();
+            Sink(o);
+        }
+
+        static void Sink(object o) { }
+    }
 }

csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected

@@ -70,6 +70,9 @@ edges
 | ExternalFlow.cs:197:42:197:43 | access to local variable o2 : Object | ExternalFlow.cs:197:18:197:44 | call to method GeneratedFlowArgs |
 | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object |
 | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs |
+| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | ExternalFlow.cs:232:21:232:21 | access to local variable h : HC |
+| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC |
+| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | ExternalFlow.cs:233:18:233:18 | access to local variable o |
 nodes
 | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
 | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
@@ -162,6 +165,10 @@ nodes
 | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
 | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | semmle.label | call to method MixedFlowArgs |
 | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
+| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | semmle.label | object creation of type HC : HC |
+| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | semmle.label | access to local variable h : HC |
+| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | semmle.label | call to method ExtensionMethod : HC |
+| ExternalFlow.cs:233:18:233:18 | access to local variable o | semmle.label | access to local variable o |
 subpaths
 #select
 | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | $@ | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | object creation of type Object : Object |
@@ -188,3 +195,4 @@ subpaths
 | ExternalFlow.cs:194:18:194:44 | call to method GeneratedFlowArgs | ExternalFlow.cs:193:22:193:33 | object creation of type Object : Object | ExternalFlow.cs:194:18:194:44 | call to method GeneratedFlowArgs | $@ | ExternalFlow.cs:193:22:193:33 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:197:18:197:44 | call to method GeneratedFlowArgs | ExternalFlow.cs:196:22:196:33 | object creation of type Object : Object | ExternalFlow.cs:197:18:197:44 | call to method GeneratedFlowArgs | $@ | ExternalFlow.cs:196:22:196:33 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | $@ | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | object creation of type Object : Object |
+| ExternalFlow.cs:233:18:233:18 | access to local variable o | ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | ExternalFlow.cs:233:18:233:18 | access to local variable o | $@ | ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | object creation of type HC : HC |

csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml

@@ -29,3 +29,4 @@ extensions:
       - ["My.Qltest", "G", false, "GeneratedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "df-generated"]
       - ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"]
       - ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "manual"]
+      - ["My.Qltest", "HE", false, "ExtensionMethod", "(My.Qltest.HI)", "", "Argument[0]", "ReturnValue", "value", "manual"]

csharp/ql/test/library-tests/dataflow/global/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +0,0 @@
-identityLocalStep
-| GlobalDataFlow.cs:573:9:576:9 | SSA phi read(f) | Node steps to itself |

csharp/ql/test/library-tests/dataflow/ssa/CONSISTENCY/DataFlowConsistency.expected

@@ -1,3 +0,0 @@
-identityLocalStep
-| DefUse.cs:80:37:80:42 | this access | Node steps to itself |
-| Properties.cs:65:24:65:31 | this access | Node steps to itself |

csharp/ql/test/query-tests/Nullness/CONSISTENCY/DataFlowConsistency.expected

@@ -1,4 +0,0 @@
-identityLocalStep
-| D.cs:320:17:320:25 | this access | Node steps to itself |
-| E.cs:123:21:123:24 | SSA phi read(x) | Node steps to itself |
-| E.cs:123:21:123:24 | SSA phi(i) | Node steps to itself |

csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/CONSISTENCY/DataFlowConsistency.expected

@@ -1,2 +0,0 @@
-identityLocalStep
-| ZipSlip.cs:13:13:45:13 | SSA phi read(destDirectory) | Node steps to itself |

csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.cs

@@ -3,6 +3,7 @@ using System.Diagnostics;
 using System.IO;
 using System.Net;
 using System.Web;
+using Microsoft.Extensions.Logging;
 
 class ILogger
 {
@@ -24,6 +25,10 @@ public class LogForgingHandler : IHttpHandler
         logger.Warn(WebUtility.HtmlEncode(username) + " logged in");
         // BAD: Logged as-is to TraceSource
         new TraceSource("Test").TraceInformation(username + " logged in");
+
+        Microsoft.Extensions.Logging.ILogger logger2 = null;
+        // BAD: Logged as-is
+        logger2.LogError(username);
     }
 
     public bool IsReusable

csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.expected

@@ -1,19 +1,23 @@
 edges
-| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:17:27:17:61 | access to indexer : String |
-| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:20:21:20:43 | ... + ... |
-| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:26:50:26:72 | ... + ... |
-| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:20:21:20:43 | ... + ... |
-| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:26:50:26:72 | ... + ... |
+| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String |
+| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:21:21:21:43 | ... + ... |
+| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:27:50:27:72 | ... + ... |
+| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:26:31:33 | access to local variable username |
+| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:21:21:21:43 | ... + ... |
+| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:27:50:27:72 | ... + ... |
+| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:31:26:31:33 | access to local variable username |
 | LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... |
 nodes
-| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
-| LogForging.cs:17:27:17:61 | access to indexer : String | semmle.label | access to indexer : String |
-| LogForging.cs:20:21:20:43 | ... + ... | semmle.label | ... + ... |
-| LogForging.cs:26:50:26:72 | ... + ... | semmle.label | ... + ... |
+| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
+| LogForging.cs:18:27:18:61 | access to indexer : String | semmle.label | access to indexer : String |
+| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
+| LogForging.cs:27:50:27:72 | ... + ... | semmle.label | ... + ... |
+| LogForging.cs:31:26:31:33 | access to local variable username | semmle.label | access to local variable username |
 | LogForgingAsp.cs:8:32:8:39 | username : String | semmle.label | username : String |
 | LogForgingAsp.cs:12:21:12:43 | ... + ... | semmle.label | ... + ... |
 subpaths
 #select
-| LogForging.cs:20:21:20:43 | ... + ... | LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:20:21:20:43 | ... + ... | This log entry depends on a $@. | LogForging.cs:17:27:17:49 | access to property QueryString | user-provided value |
-| LogForging.cs:26:50:26:72 | ... + ... | LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:26:50:26:72 | ... + ... | This log entry depends on a $@. | LogForging.cs:17:27:17:49 | access to property QueryString | user-provided value |
+| LogForging.cs:21:21:21:43 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:21:21:21:43 | ... + ... | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
+| LogForging.cs:27:50:27:72 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:27:50:27:72 | ... + ... | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
+| LogForging.cs:31:26:31:33 | access to local variable username | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:26:31:33 | access to local variable username | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
 | LogForgingAsp.cs:12:21:12:43 | ... + ... | LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... | This log entry depends on a $@. | LogForgingAsp.cs:8:32:8:39 | username | user-provided value |

docs/codeql/codeql-language-guides/codeql-library-for-javascript.rst

@@ -193,7 +193,7 @@ The class `ASTNode <https://codeql.github.com/codeql-standard-libraries/javascri
 .. pull-quote::
 
    Note
-   
+
    These predicates should only be used to perform generic AST traversal. To access children of specific AST node types, the specialized predicates introduced below should be used instead. In particular, queries should not rely on the numeric indices of child nodes relative to their parent nodes: these are considered an implementation detail that may change between versions of the library.
 
 Top-levels
@@ -700,28 +700,7 @@ The data flow graph-based analyses described so far are all intraprocedural: the
 
 We distinguish here between data flow proper, and *taint tracking*: the latter not only considers value-preserving flow (such as from variable definitions to uses), but also cases where one value influences ("taints") another without determining it entirely. For example, in the assignment ``s2 = s1.substring(i)``, the value of ``s1`` influences the value of ``s2``, because ``s2`` is assigned a substring of ``s1``. In general, ``s2`` will not be assigned ``s1`` itself, so there is no data flow from ``s1`` to ``s2``, but ``s1`` still taints ``s2``.
 
-The simplest way of implementing an interprocedural data flow analysis is to extend either class ``DataFlow::TrackedNode`` or ``DataFlow::TrackedExpr``. The former is a subclass of ``DataFlow::Node``, the latter of ``Expr``, and extending them ensures that the newly added values are tracked interprocedurally. You can use the predicate ``flowsTo`` to find out which nodes/expressions the tracked value flows to.
-
-For example, suppose that we are developing an analysis to find hard-coded passwords. We might start by writing a simple query that looks for string constants flowing into variables named ``"password"``. To do this, we can extend ``TrackedExpr`` to track all constant strings, ``flowsTo`` to find cases where such a string flows into a (SSA) definition of a password variable:
-
-.. code-block:: ql
-
-   import javascript
-
-   class TrackedStringLiteral extends DataFlow::TrackedNode {
-       TrackedStringLiteral() {
-           this.asExpr() instanceof ConstantString
-       }
-   }
-
-   from TrackedStringLiteral source, DataFlow::Node sink, SsaExplicitDefinition def
-   where source.flowsTo(sink) and sink = DataFlow::ssaDefinitionNode(def) and
-         def.getSourceVariable().getName().toLowerCase() = "password"
-   select sink
-
-Note that ``TrackedNode`` and ``TrackedExpr`` do not restrict the set of "sinks" for the inter-procedural flow analysis, tracking flow into any expression that they might flow to. This can be expensive for large code bases, and is often unnecessary, since usually you are only interested in flow to a particular set of sinks. For example, the above query only looks for flow into assignments to password variables.
-
-This is a particular instance of a general pattern, whereby we want to specify a data flow or taint analysis in terms of its *sources* (where flow starts), *sinks* (where it should be tracked), and *barriers* or *sanitizers* (where flow is interrupted). The example does not include any sanitizers, but they are very common in security analyses: for example, an analysis that tracks the flow of untrusted user input into, say, a SQL query has to keep track of code that validates the input, thereby making it safe to use. Such a validation step is an example of a sanitizer.
+It is a common pattern that we wish to specify data flow or taint analysis in terms of its *sources* (where flow starts), *sinks* (where it should be tracked), and *barriers* or *sanitizers* (where flow is interrupted). Sanitizers they are very common in security analyses: for example, an analysis that tracks the flow of untrusted user input into, say, a SQL query has to keep track of code that validates the input, thereby making it safe to use. Such a validation step is an example of a sanitizer.
 
 The classes ``DataFlow::Configuration`` and ``TaintTracking::Configuration`` allow specifying a data flow or taint analysis, respectively, by overriding the following predicates:
 
@@ -735,10 +714,12 @@ Since for technical reasons both ``Configuration`` classes are subtypes of ``str
 
 The predicate ``Configuration.hasFlow`` performs the actual flow tracking, starting at a source and looking for flow to a sink that does not pass through a barrier node or edge.
 
-To continue with our above example, we can phrase it as a data flow configuration as follows:
+For example, suppose that we are developing an analysis to find hard-coded passwords. We might write a simple query that looks for string constants flowing into variables named ``"password"``.
 
 .. code-block:: ql
 
+   import javascript
+
    class PasswordTracker extends DataFlow::Configuration {
        PasswordTracker() {
            // unique identifier for this configuration
@@ -754,11 +735,8 @@ To continue with our above example, we can phrase it as a data flow configuratio
        }
 
        predicate passwordVarAssign(Variable v, DataFlow::Node nd) {
-           exists (SsaExplicitDefinition def |
-               nd = DataFlow::ssaDefinitionNode(def) and
-               def.getSourceVariable() = v and
-               v.getName().toLowerCase() = "password"
-           )
+          v.getAnAssignedExpr() = nd.asExpr() and
+          v.getName().toLowerCase() = "password"
        }
    }
 
@@ -770,7 +748,6 @@ Now we can rephrase our query to use ``Configuration.hasFlow``:
    where pt.hasFlow(source, sink) and pt.passwordVarAssign(v, sink)
    select sink, "Password variable " + v + " is assigned a constant string."
 
-Note that while analyses implemented in this way are inter-procedural in that they track flow and taint across function calls and returns, flow through global variables is not tracked. Flow through object properties is only tracked in limited cases, for example through properties of object literals or CommonJS ``module`` and ``exports`` objects.
 
 Syntax errors
 ~~~~~~~~~~~~~
@@ -794,7 +771,7 @@ The ``semmle.javascript.frameworks.AngularJS`` library provides support for work
 HTTP framework libraries
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
-The library ``semmle.javacript.frameworks.HTTP`` provides classes modeling common concepts from various HTTP frameworks. 
+The library ``semmle.javacript.frameworks.HTTP`` provides classes modeling common concepts from various HTTP frameworks.
 
 Currently supported frameworks are `Express <https://expressjs.com/>`__, the standard Node.js ``http`` and ``https`` modules, `Connect <https://github.com/senchalabs/connect>`__, `Koa <https://koajs.com>`__, `Hapi <https://hapi.dev/>`__ and `Restify <http://restify.com/>`__.
 

docs/codeql/ql-language-reference/ql-language-specification.rst

@@ -176,9 +176,13 @@ A QL module definition has the following syntax:
 
 ::
 
-   module ::= annotation* "module" modulename "{" moduleBody "}"
+   module ::= annotation* "module" modulename parameters? implements? "{" moduleBody "}"
 
-   moduleBody ::= (import | predicate | class | module | alias | select)*
+   parameters ::= "<" signatureExpr parameterName ("," signatureExpr parameterName)* ">"
+
+   implements ::= "implements" moduleSignatureExpr ("," moduleSignatureExpr)*
+
+   moduleBody ::= (import | predicate | class | module | signature | alias | select)*
 
 A module definition extends the current module's declared module environment with a mapping from the module name to the module definition.
 
@@ -208,12 +212,15 @@ An import directive refers to a module identifier:
 
 ::
 
-   import ::= annotations "import" importModuleId ("as" modulename)?
+   import ::= annotations "import" importModuleExpr ("as" modulename)?
 
    qualId ::= simpleId | qualId "." simpleId
 
-   importModuleId ::= qualId
-                  | importModuleId "::" simpleId
+   importModuleExpr ::= qualId | importModuleExpr "::" modulename arguments?
+
+   arguments ::= "<" argument ("," argument)* ">"
+
+   argument ::= moduleExpr | type | predicateRef "/" int
 
 An import directive may optionally name the imported module using an ``as`` declaration. If a name is defined, then the import directive adds to the declared module environment of the current module a mapping from the name to the declaration of the imported module. Otherwise, the current module *directly imports* the imported module.
 
@@ -280,9 +287,9 @@ With the exception of class domain types and character types (which cannot be re
 
 ::
 
-   type ::= (moduleId "::")? classname | dbasetype | "boolean" | "date" | "float" | "int" | "string"
+   type ::= (moduleExpr "::")? classname | dbasetype | "boolean" | "date" | "float" | "int" | "string"
 
-   moduleId ::= simpleId | moduleId "::" simpleId
+   moduleExpr ::= modulename arguments? | moduleExpr "::" modulename arguments?
 
 A type reference is resolved to a type as follows:
 
@@ -327,6 +334,40 @@ Active types
 
 In a QL program, the *active* types are those defined in active modules. In the remainder of this specification, any reference to the types in the program refers only to the active types.
 
+
+Signatures
+----------
+
+Signature definitions
+~~~~~~~~~~~~~~~~~~~~~
+
+A QL signature definition has the following syntax:
+
+::
+
+   signature ::= predicateSignature | typeSignature | moduleSignature
+
+   predicateSignature ::= qldoc? annotations "signature" head ";"
+
+   typeSignature ::= qldoc? annotations "signature" "class" classname ("extends" type ("," type)*)? (";" | "{" signaturePredicate* "}")
+
+   moduleSignature ::= qldoc? annotation* "signature" "module" moduleSignatureName parameters? "{" moduleSignatureBody "}"
+
+   moduleSignatureBody ::= (signaturePredicate | defaultPredicate | signatureType)*
+
+   signaturePredicate ::= qldoc? annotations head ";"
+
+   defaultPredicate ::= qldoc? annotations "default" head "{" formula "}"
+
+   signatureType ::= qldoc? annotations "class" classname ("extends" type ("," type)*)? "{" signaturePredicate* "}"
+
+
+A predicate signature definition extends the current module's declared predicate signature environment with a mapping from the predicate signature name and arity to the predicate signature definition.
+
+A type signature definition extends the current module's declared type signature environment with a mapping from the type signature name to the type signature definition.
+
+A module signature definition extends the current module's declared module signature environment with a mapping from the module signature name to the module signature definition.
+
 Values
 ------
 
@@ -587,20 +628,21 @@ There are several kinds of identifiers:
 
 -  ``atLowerId``: an identifier that starts with an "@" sign and then a lower-case letter.
 
--  ``atUpperId``: an identifier that starts with an "@" sign and then an upper-case letter.
-
 Identifiers are used in following syntactic constructs:
 
 ::
 
-   simpleId      ::= lowerId | upperId
-   modulename    ::= simpleId
-   classname     ::= upperId
-   dbasetype     ::= atLowerId
-   predicateRef  ::= (moduleId "::")? literalId
-   predicateName ::= lowerId
-   varname       ::= lowerId
-   literalId     ::= lowerId | atLowerId
+   simpleId            ::= lowerId | upperId
+   modulename          ::= simpleId
+   moduleSignatureName ::= upperId
+   classname           ::= upperId
+   dbasetype           ::= atLowerId
+   predicateRef        ::= (moduleExpr "::")? literalId
+   signatureExpr       ::= (moduleExpr "::")? simpleId ("/" Integer | arguments)?;
+   predicateName       ::= lowerId
+   parameterName       ::= simpleId
+   varname             ::= lowerId
+   literalId           ::= lowerId | atLowerId
 
 Integer literals (int)
 ~~~~~~~~~~~~~~~~~~~~~~
@@ -1615,7 +1657,7 @@ Aliases define new names for existing QL entities.
 
    alias ::= qldoc? annotations "predicate" literalId "=" predicateRef "/" int ";"
          |   qldoc? annotations "class" classname "=" type ";"
-         |   qldoc? annotations "module" modulename "=" moduleId ";"
+         |   qldoc? annotations "module" modulename "=" moduleExpr ";"
 
 
 An alias introduces a binding from the new name to the entity referred to by the right-hand side in the current module's declared predicate, type, or module environment respectively.
@@ -2064,16 +2106,39 @@ The complete grammar for QL is as follows:
 
    ql ::= qldoc? moduleBody
 
-   module ::= annotation* "module" modulename "{" moduleBody "}"
+   module ::= annotation* "module" modulename parameters? implements? "{" moduleBody "}"
+
+   parameters ::= "<" signatureExpr parameterName ("," signatureExpr parameterName)* ">"
+
+   implements ::= "implements" moduleSignatureExpr ("," moduleSignatureExpr)*
 
    moduleBody ::= (import | predicate | class | module | alias | select)*
 
-   import ::= annotations "import" importModuleId ("as" modulename)?
+   import ::= annotations "import" importModuleExpr ("as" modulename)?
 
    qualId ::= simpleId | qualId "." simpleId
 
-   importModuleId ::= qualId
-                  | importModuleId "::" simpleId
+   importModuleExpr ::= qualId | importModuleExpr "::" modulename arguments?
+
+   arguments ::= "<" argument ("," argument)* ">"
+
+   argument ::= moduleExpr | type | predicateRef "/" int
+
+   signature ::= predicateSignature | typeSignature | moduleSignature
+
+   predicateSignature ::= qldoc? annotations "signature" head ";"
+
+   typeSignature ::= qldoc? annotations "signature" "class" classname ("extends" type ("," type)*)? (";" | "{" signaturePredicate* "}")
+
+   moduleSignature ::= qldoc? annotation* "signature" "module" moduleSignatureName parameters? "{" moduleSignatureBody "}"
+
+   moduleSignatureBody ::= (signaturePredicate | defaultPredicate | signatureType)*
+
+   signaturePredicate ::= qldoc? annotations head ";"
+
+   defaultPredicate ::= qldoc? annotations "default" head "{" formula "}"
+
+   signatureType ::= qldoc? annotations "class" classname ("extends" type ("," type)*)? "{" signaturePredicate* "}"
 
    select ::= ("from" var_decls)? ("where" formula)? "select" as_exprs ("order" "by" orderbys)?
 
@@ -2120,15 +2185,19 @@ The complete grammar for QL is as follows:
 
    field ::= qldoc? annotations var_decl ";"
 
-   moduleId ::= simpleId | moduleId "::" simpleId
+   moduleExpr ::= modulename arguments? | moduleExpr "::" modulename arguments?
 
-   type ::= (moduleId "::")? classname | dbasetype | "boolean" | "date" | "float" | "int" | "string"
+   moduleSignatureExpr ::= (moduleExpr "::")? moduleSignatureName arguments?
+
+   signatureExpr : (moduleExpr "::")? simpleId ("/" Integer | arguments)?;
+
+   type ::= (moduleExpr "::")? classname | dbasetype | "boolean" | "date" | "float" | "int" | "string"
 
    exprs ::= expr ("," expr)*
 
    alias ::= qldoc? annotations "predicate" literalId "=" predicateRef "/" int ";"
          |  qldoc? annotations "class" classname "=" type ";"
-         |  qldoc? annotations "module" modulename "=" moduleId ";"
+         |  qldoc? annotations "module" modulename "=" moduleExpr ";"
 
    var_decls ::= (var_decl ("," var_decl)*)?
 
@@ -2245,14 +2314,18 @@ The complete grammar for QL is as follows:
 
    modulename ::= simpleId
 
+   moduleSignatureName ::= upperId
+
    classname ::= upperId
 
    dbasetype ::= atLowerId
 
-   predicateRef ::= (moduleId "::")? literalId
+   predicateRef ::= (moduleExpr "::")? literalId
 
    predicateName ::= lowerId
 
+   parameterName ::= simpleId
+
    varname ::= lowerId
 
    literalId ::= lowerId | atLowerId | "any" | "none"

docs/codeql/reusables/supported-versions-compilers.rst

@@ -4,7 +4,7 @@
    :stub-columns: 1
 
    Language,Variants,Compilers,Extensions
-   C/C++,"C89, C99, C11, C18, C++98, C++03, C++11, C++14, C++17, C++20 [1]_","Clang (and clang-cl [2]_) extensions (up to Clang 12.0),
+   C/C++,"C89, C99, C11, C17, C++98, C++03, C++11, C++14, C++17, C++20 [1]_","Clang (and clang-cl [2]_) extensions (up to Clang 12.0),
 
    GNU extensions (up to GCC 11.1),
 

go/Makefile

@@ -113,9 +113,9 @@ ql/lib/go.dbscheme.stats: ql/lib/go.dbscheme build/stats/src.stamp extractor
 
 test: all build/testdb/check-upgrade-path
 	codeql test run -j0 ql/test --search-path build/codeql-extractor-go --consistency-queries ql/test/consistency --compilation-cache=$(cache)
-  #	use GOOS=linux because GOOS=darwin GOARCH=386 is no longer supported
+#	use GOOS=linux because GOOS=darwin GOARCH=386 is no longer supported
 	env GOOS=linux GOARCH=386 codeql$(EXE) test run -j0 ql/test/query-tests/Security/CWE-681 --search-path build/codeql-extractor-go --consistency-queries ql/test/consistency --compilation-cache=$(cache)
-	cd extractor; go test -mod=vendor ./... | grep -vF "[no test files]"
+	cd extractor; go test -mod=vendor ./...
 	bash extractor-smoke-test/test.sh || (echo "Extractor smoke test FAILED"; exit 1)
 
 .PHONY: build/testdb/check-upgrade-path

go/extractor/cli/go-autobuilder/go-autobuilder.go

@@ -25,10 +25,8 @@ func usage() {
 
 Options:
   --identify-environment
-    Produce an environment file specifying which Go version should be installed in the environment
-	so that autobuilding will be successful. The location of this file is controlled by the
-    environment variable CODEQL_EXTRACTOR_ENVIRONMENT_JSON, or defaults to 'environment.json' if
-	that is not set.
+    Output some json on stdout specifying which Go version should be installed in the environment
+	so that autobuilding will be successful.
 
 Build behavior:
 
@@ -745,8 +743,8 @@ func getVersionWhenGoModVersionNotFound(v versionInfo) (msg, version string) {
 		// There is no Go version installed in the environment. We have no indication which version
 		// was intended to be used to build this project. Go versions are generally backwards
 		// compatible, so we install the maximum supported version.
-		msg = "No version of Go installed and no `go.mod` file found. Writing an environment " +
-			"file specifying the maximum supported version of Go (" + maxGoVersion + ")."
+		msg = "No version of Go installed and no `go.mod` file found. Requesting the maximum " +
+			"supported version of Go (" + maxGoVersion + ")."
 		version = maxGoVersion
 		diagnostics.EmitNoGoModAndNoGoEnv(msg)
 	} else if outsideSupportedRange(v.goEnvVersion) {
@@ -928,39 +926,19 @@ func getVersionToInstall(v versionInfo) (msg, version string) {
 	return getVersionWhenGoModVersionSupported(v)
 }
 
-// Write an environment file to the current directory. If `version` is the empty string then
-// write an empty environment file, otherwise write an environment file specifying the version
-// of Go to install. The path to the environment file is specified by the
-// CODEQL_EXTRACTOR_ENVIRONMENT_JSON environment variable, or defaults to `environment.json`.
-func writeEnvironmentFile(version string) {
+// Output some JSON to stdout specifying the version of Go to install, unless `version` is the
+// empty string.
+func outputEnvironmentJson(version string) {
 	var content string
 	if version == "" {
 		content = `{ "include": [] }`
 	} else {
 		content = `{ "include": [ { "go": { "version": "` + version + `" } } ] }`
 	}
+	_, err := fmt.Fprint(os.Stdout, content)
 
-	filename, ok := os.LookupEnv("CODEQL_EXTRACTOR_ENVIRONMENT_JSON")
-	if !ok {
-		filename = "environment.json"
-	}
-
-	targetFile, err := os.Create(filename)
 	if err != nil {
-		log.Println("Failed to create environment file " + filename + ": ")
-		log.Println(err)
-		return
-	}
-	defer func() {
-		if err := targetFile.Close(); err != nil {
-			log.Println("Failed to close environment file " + filename + ":")
-			log.Println(err)
-		}
-	}()
-
-	_, err = targetFile.WriteString(content)
-	if err != nil {
-		log.Println("Failed to write to environment file " + filename + ": ")
+		log.Println("Failed to write environment json to stdout: ")
 		log.Println(err)
 	}
 }
@@ -984,7 +962,7 @@ func isGoInstalled() bool {
 	return err == nil
 }
 
-// Get the version of Go to install and write it to an environment file.
+// Get the version of Go to install and output it to stdout as json.
 func identifyEnvironment() {
 	var v versionInfo
 	depMode := getDepMode()
@@ -998,7 +976,7 @@ func identifyEnvironment() {
 	msg, versionToInstall := getVersionToInstall(v)
 	log.Println(msg)
 
-	writeEnvironmentFile(versionToInstall)
+	outputEnvironmentJson(versionToInstall)
 }
 
 func main() {

go/extractor/cli/go-autobuilder/go-autobuilder_test.go

@@ -36,45 +36,37 @@ func TestParseGoVersion(t *testing.T) {
 
 func TestGetVersionToInstall(t *testing.T) {
 	tests := map[versionInfo]string{
-		// checkForUnsupportedVersions()
-
-		// go.mod version below minGoVersion
-		{"0.0", true, "1.20.3", true}: "",
-		{"0.0", true, "9999.0", true}: "",
-		{"0.0", true, "1.2.2", true}:  "",
-		{"0.0", true, "", false}:      "",
-		// go.mod version above maxGoVersion
-		{"9999.0", true, "1.20.3", true}:   "",
-		{"9999.0", true, "9999.0.1", true}: "",
-		{"9999.0", true, "1.1", true}:      "",
-		{"9999.0", true, "", false}:        "",
-		// Go installation found with version below minGoVersion
-		{"1.20", true, "1.2.2", true}: "1.20",
-		{"1.11", true, "1.2.2", true}: "1.11",
+		// getVersionWhenGoModVersionNotFound()
+		{"", false, "", false}:        maxGoVersion,
 		{"", false, "1.2.2", true}:    maxGoVersion,
-		// Go installation found with version above maxGoVersion
+		{"", false, "9999.0.1", true}: maxGoVersion,
+		{"", false, "1.11.13", true}:  "",
+		{"", false, "1.20.3", true}:   "",
+
+		// getVersionWhenGoModVersionTooHigh()
+		{"9999.0", true, "", false}:           maxGoVersion,
+		{"9999.0", true, "9999.0.1", true}:    "",
+		{"9999.0", true, "1.1", true}:         maxGoVersion,
+		{"9999.0", true, minGoVersion, false}: maxGoVersion,
+		{"9999.0", true, maxGoVersion, true}:  "",
+
+		// getVersionWhenGoModVersionTooLow()
+		{"0.0", true, "", false}:      minGoVersion,
+		{"0.0", true, "9999.0", true}: minGoVersion,
+		{"0.0", true, "1.2.2", true}:  minGoVersion,
+		{"0.0", true, "1.20.3", true}: "",
+
+		// getVersionWhenGoModVersionSupported()
+		{"1.20", true, "", false}:        "1.20",
+		{"1.11", true, "", false}:        "1.11",
+		{"1.20", true, "1.2.2", true}:    "1.20",
+		{"1.11", true, "1.2.2", true}:    "1.11",
 		{"1.20", true, "9999.0.1", true}: "1.20",
 		{"1.11", true, "9999.0.1", true}: "1.11",
-		{"", false, "9999.0.1", true}:    maxGoVersion,
-
-		// checkForVersionsNotFound()
-
-		// Go installation not found, go.mod version in supported range
-		{"1.20", true, "", false}: "1.20",
-		{"1.11", true, "", false}: "1.11",
-		// Go installation not found, go.mod not found
-		{"", false, "", false}: maxGoVersion,
-		// Go installation found with version in supported range, go.mod not found
-		{"", false, "1.11.13", true}: "",
-		{"", false, "1.20.3", true}:  "",
-
-		// compareVersions()
-
-		// Go installation found with version in supported range, go.mod version in supported range and go.mod version > go installation version
+		// go.mod version > go installation version
 		{"1.20", true, "1.11.13", true}: "1.20",
 		{"1.20", true, "1.12", true}:    "1.20",
-		// Go installation found with version in supported range, go.mod version in supported range and go.mod version <= go installation version
-		// (Note comparisons ignore the patch version)
+		// go.mod version <= go installation version (Note comparisons ignore the patch version)
 		{"1.11", true, "1.20", true}:   "",
 		{"1.11", true, "1.20.3", true}: "",
 		{"1.20", true, "1.20.3", true}: "",

go/ql/lib/semmle/go/security/SensitiveActions.qll

@@ -35,7 +35,7 @@ module HeuristicNames {
    */
   string maybePassword() {
     result = "(?is).*pass(wd|word|code|phrase)(?!.*question).*" or
-    result = "(?is).*(auth(entication|ori[sz]ation)?)key.*"
+    result = "(?is).*(auth(entication|ori[sz]ation)?|api)key.*"
   }
 
   /**

go/ql/src/RedundantCode/DeadStoreOfField.ql

@@ -36,7 +36,10 @@ predicate escapes(DataFlow::Node nd) {
   exists(SendStmt s | nd.asExpr() = s.getValue())
   or
   // if `nd` is passed to a function, then it escapes
-  nd instanceof DataFlow::ArgumentNode
+  nd = any(DataFlow::CallNode c).getASyntacticArgument()
+  or
+  // if `nd` is the receiver of a function, then it escapes
+  nd = any(DataFlow::MethodCallNode c).getReceiver()
   or
   // if `nd` has its address taken, then it escapes
   exists(AddressExpr ae | nd.asExpr() = ae.getOperand())

go/ql/src/experimental/CWE-134/DsnBad.go

@@ -0,0 +1,8 @@
+
+func bad() interface{} {
+	name := os.Args[1:]
+	// This is bad. `name` can be something like `test?allowAllFiles=true&` which will allow an attacker to access local files.
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db
+}

go/ql/src/experimental/CWE-134/DsnGood.go

@@ -0,0 +1,12 @@
+func good() (interface{}, error) {
+	name := os.Args[1]
+	hasBadChar, _ := regexp.MatchString(".*[?].*", name)
+
+	if hasBadChar {
+		return nil, errors.New("Bad input")
+	}
+
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db, nil
+}

go/ql/src/experimental/CWE-134/DsnInjection.qhelp

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+
+<qhelp>
+  <overview>
+    <p>If a Data-Source Name (DSN) is built using untrusted user input without proper sanitization,
+      the system may be vulnerable to DSN injection vulnerabilities.</p>
+  </overview>
+
+  <recommendation>
+    <p>If user input must be included in a DSN, additional steps should be taken to sanitize
+      untrusted data, such as checking for special characters included in user input.</p>
+  </recommendation>
+
+  <example>
+    <p>In the following examples, the code accepts the db name from the user,
+      which it then uses to build a DSN string.</p>
+
+    <p>The following example uses the unsanitized user input directly
+      in the process of constructing a DSN name.
+      A malicious user could provide special characters to change the meaning of this string, and
+      carry out unexpected database operations.</p>
+
+    <sample src="DsnBad.go" />
+
+    <p>In the following example, the input provided by the user is sanitized before it is included
+      in the DSN string.
+      This ensures the meaning of the DSN string cannot be changed by a malicious user.</p>
+
+    <sample src="DsnGood.go" />
+  </example>
+
+  <references>
+    <li>
+      CVE-2022-3023: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-3023/">Data Source Name Injection in pingcap/tidb.</a>
+    </li>
+
+  </references>
+</qhelp>

go/ql/src/experimental/CWE-134/DsnInjection.ql

@@ -0,0 +1,22 @@
+/**
+ * @name SQL Data-source URI built from user-controlled sources
+ * @description Building an SQL data-source URI from untrusted sources can allow attacker to compromise security
+ * @kind path-problem
+ * @problem.severity error
+ * @id go/dsn-injection
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-134
+ */
+
+import go
+import DataFlow::PathGraph
+import DsnInjectionCustomizations
+
+/** An untrusted flow source taken as a source for the `DsnInjection` taint-flow configuration. */
+private class UntrustedFlowAsSource extends Source instanceof UntrustedFlowSource { }
+
+from DsnInjection cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
+  "user-provided value"

go/ql/src/experimental/CWE-134/DsnInjectionCustomizations.qll

@@ -0,0 +1,46 @@
+/** Provides a taint-tracking model to reason about Data-Source name injection vulnerabilities. */
+
+import go
+import DataFlow::PathGraph
+import semmle.go.dataflow.barrierguardutil.RegexpCheck
+
+/** A source for `DsnInjection` taint-flow configuration. */
+abstract class Source extends DataFlow::Node { }
+
+/** A taint-tracking configuration to reason about Data Source Name injection vulnerabilities. */
+class DsnInjection extends TaintTracking::Configuration {
+  DsnInjection() { this = "DsnInjection" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof Source }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(Function f | f.hasQualifiedName("database/sql", "Open") |
+      node = f.getACall().getArgument(1)
+    )
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof RegexpCheckBarrier }
+}
+
+/** A model of a function which decodes or unmarshals a tainted input, propagating taint from any argument to either the method receiver or return value. */
+private class DecodeFunctionModel extends TaintTracking::FunctionModel {
+  DecodeFunctionModel() {
+    // This matches any function with a name like `Decode`,`Unmarshal` or `Parse`.
+    // This is done to allow taints stored in encoded forms, such as in toml or json to flow freely.
+    this.getName().regexpMatch("(?i).*(parse|decode|unmarshal).*")
+  }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameter(_) and
+    (output.isResult(0) or output.isReceiver())
+  }
+}
+
+/** A model of `flag.Parse`, propagating tainted input passed via CLI flags to `Parse`'s result. */
+private class FlagSetFunctionModel extends TaintTracking::FunctionModel {
+  FlagSetFunctionModel() { this.hasQualifiedName("flag", "Parse") }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameter(0) and output.isResult()
+  }
+}

go/ql/src/experimental/CWE-134/DsnInjectionLocal.ql

@@ -0,0 +1,24 @@
+/**
+ * @name SQL Data-source URI built from local user-controlled sources
+ * @description Building an SQL data-source URI from untrusted sources can allow attacker to compromise security
+ * @kind path-problem
+ * @problem.severity error
+ * @id go/dsn-injection-local
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-134
+ */
+
+import go
+import DataFlow::PathGraph
+import DsnInjectionCustomizations
+
+/** An argument passed via the command line taken as a source for the `DsnInjection` taint-flow configuration. */
+private class OsArgsSource extends Source {
+  OsArgsSource() { this = any(Variable c | c.hasQualifiedName("os", "Args")).getARead() }
+}
+
+from DsnInjection cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
+  "user-provided value"

go/ql/src/experimental/CWE-203/Timing.qhelp

@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      Using a non-constant time comparision to compare sensitive information can lead to auth
+      vulnerabilities.
+</p>
+  </overview>
+
+  <recommendation>
+    <p>Use of a constant time comparision function such as <code>crypto/subtle</code> package's <code>
+      ConstantTimeCompare</code> function can prevent this vulnerability. </p>
+  </recommendation>
+
+  <example>
+    <p>In the following examples, the code accepts a secret via a HTTP header in variable <code>
+      secretHeader</code> and a secret from the user in the <code>headerSecret</code> variable, which
+      are then compared with a system stored secret to perform authentication.</p>
+
+
+    <sample src="timingBad.go" />
+
+    <p>In the following example, the input provided by the user is compared using the <code>
+      ConstantTimeComapre</code> function. This ensures that timing attacks are not possible in this
+      case.</p>
+
+    <sample src="timingGood.go" />
+  </example>
+
+  <references>
+    <li>National Vulnerability Database: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-24912">
+      CVE-2022-24912</a>.</li>
+    <li>Verbose Logging:<a href="https://verboselogging.com/2012/08/20/a-timing-attack-in-action"> A
+      timing attack in action </a></li>
+  </references>
+</qhelp>

go/ql/src/experimental/CWE-203/Timing.ql

@@ -0,0 +1,72 @@
+/**
+ * @name Timing attacks due to comparison of sensitive secrets
+ * @description using a non-constant time comparison method to compare secrets can lead to authoriztion vulnerabilities
+ * @kind path-problem
+ * @problem.severity warning
+ * @id go/timing-attack
+ * @tags security
+ *       experimental
+ *       external/cwe/cwe-203
+ */
+
+import go
+import DataFlow::PathGraph
+import semmle.go.security.SensitiveActions
+
+private predicate isBadResult(DataFlow::Node e) {
+  exists(string path | path = e.asExpr().getFile().getAbsolutePath().toLowerCase() |
+    path.matches(["%fake%", "%dummy%", "%test%", "%example%"]) and not path.matches("%ql/test%")
+  )
+}
+
+/**
+ * A data flow sink for timing attack vulnerabilities.
+ */
+abstract class Sink extends DataFlow::Node { }
+
+/** A taint-tracking sink which models comparisons of sensitive variables. */
+private class SensitiveCompareSink extends Sink {
+  ComparisonExpr c;
+
+  SensitiveCompareSink() {
+    // We select a comparison where a secret or password is tested.
+    exists(SensitiveVariableAccess op1, Expr op2 |
+      op1.getClassification() = [SensitiveExpr::secret(), SensitiveExpr::password()] and
+      // exclude grant to avoid FP from OAuth
+      not op1.getClassification().matches("%grant%") and
+      op1 = c.getAnOperand() and
+      op2 = c.getAnOperand() and
+      not op1 = op2 and
+      not (
+        // Comparisons with `nil` should be excluded.
+        op2 = Builtin::nil().getAReference()
+        or
+        // Comparisons with empty string should also be excluded.
+        op2.getStringValue().length() = 0
+      )
+    |
+      // It is important to note that the name of both the operands need not be
+      // `sensitive`. Even if one of the operands appears to be sensitive, we consider it a potential sink.
+      c.getAnOperand() = this.asExpr()
+    )
+  }
+
+  DataFlow::Node getOtherOperand() { result.asExpr() = c.getAnOperand() and not result = this }
+}
+
+class SecretTracking extends TaintTracking::Configuration {
+  SecretTracking() { this = "SecretTracking" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source instanceof UntrustedFlowSource and not isBadResult(source)
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink and not isBadResult(sink) }
+}
+
+from SecretTracking cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where
+  cfg.hasFlowPath(source, sink) and
+  not cfg.hasFlowTo(sink.getNode().(SensitiveCompareSink).getOtherOperand())
+select sink.getNode(), source, sink, "$@ may be vulnerable to timing attacks.", source.getNode(),
+  "Hardcoded String"

go/ql/src/experimental/CWE-203/timingBad.go

@@ -0,0 +1,11 @@
+func bad(w http.ResponseWriter, req *http.Request, []byte secret) (interface{}, error) {
+
+	secretHeader := "X-Secret"
+
+	headerSecret := req.Header.Get(secretHeader)
+	secretStr := string(secret)
+	if len(secret) != 0 && headerSecret != secretStr {
+		return nil, fmt.Errorf("header %s=%s did not match expected secret", secretHeader, headerSecret)
+	}
+	return nil, nil
+}

go/ql/src/experimental/CWE-203/timingGood.go

@@ -0,0 +1,10 @@
+func good(w http.ResponseWriter, req *http.Request, []byte secret) (interface{}, error) {
+
+	secretHeader := "X-Secret"
+
+	headerSecret := req.Header.Get(secretHeader)
+	if len(secret) != 0 && subtle.ConstantTimeCompare(secret, []byte(headerSecret)) != 1 {
+		return nil, fmt.Errorf("header %s=%s did not match expected secret", secretHeader, headerSecret)
+	}
+	return nil, nil
+}

go/ql/test/TestUtilities/InlineFlowTest.qll

@@ -78,7 +78,7 @@ class InlineFlowTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasValueFlow" and
-    exists(DataFlow::Node sink | getValueFlowConfig().hasFlowTo(sink) |
+    exists(DataFlow::Node sink | this.getValueFlowConfig().hasFlowTo(sink) |
       sink.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
         location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
       element = sink.toString() and
@@ -87,7 +87,8 @@ class InlineFlowTest extends InlineExpectationsTest {
     or
     tag = "hasTaintFlow" and
     exists(DataFlow::Node src, DataFlow::Node sink |
-      getTaintFlowConfig().hasFlow(src, sink) and not getValueFlowConfig().hasFlow(src, sink)
+      this.getTaintFlowConfig().hasFlow(src, sink) and
+      not this.getValueFlowConfig().hasFlow(src, sink)
     |
       sink.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
         location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and

go/ql/test/experimental/CWE-134/Dsn.go

@@ -0,0 +1,77 @@
+package main
+
+import (
+	"database/sql"
+	"errors"
+	"fmt"
+	"net/http"
+	"os"
+	"regexp"
+)
+
+func good() (interface{}, error) {
+	name := os.Args[1]
+	hasBadChar, _ := regexp.MatchString(".*[?].*", name)
+
+	if hasBadChar {
+		return nil, errors.New("bad input")
+	}
+
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db, nil
+}
+
+func bad() interface{} {
+	name2 := os.Args[1:]
+	// This is bad. `name` can be something like `test?allowAllFiles=true&` which will allow an attacker to access local files.
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name2[0])
+	db, _ := sql.Open("mysql", dbDSN)
+	return db
+}
+
+func good2(w http.ResponseWriter, req *http.Request) (interface{}, error) {
+	name := req.FormValue("name")
+	hasBadChar, _ := regexp.MatchString(".*[?].*", name)
+
+	if hasBadChar {
+		return nil, errors.New("bad input")
+	}
+
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db, nil
+}
+
+func bad2(w http.ResponseWriter, req *http.Request) interface{} {
+	name := req.FormValue("name")
+	// This is bad. `name` can be something like `test?allowAllFiles=true&` which will allow an attacker to access local files.
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, name)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db
+}
+
+type Config struct {
+	dsn string
+}
+
+func NewConfig() *Config            { return &Config{dsn: ""} }
+func (Config) Parse([]string) error { return nil }
+
+func RegexFuncModelTest(w http.ResponseWriter, req *http.Request) (interface{}, error) {
+	cfg := NewConfig()
+	err := cfg.Parse(os.Args[1:]) // This is bad. `name` can be something like `test?allowAllFiles=true&` which will allow an attacker to access local files.
+	if err != nil {
+		return nil, err
+	}
+	dbDSN := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?charset=utf8", "username", "password", "127.0.0.1", 3306, cfg.dsn)
+	db, _ := sql.Open("mysql", dbDSN)
+	return db, nil
+}
+
+func main() {
+	bad2(nil, nil)
+	good()
+	bad()
+	good2(nil, nil)
+}

go/ql/test/experimental/CWE-134/DsnInjection.expected

@@ -0,0 +1,8 @@
+edges
+| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:50:29:50:33 | dbDSN |
+nodes
+| Dsn.go:47:10:47:30 | call to FormValue | semmle.label | call to FormValue |
+| Dsn.go:50:29:50:33 | dbDSN | semmle.label | dbDSN |
+subpaths
+#select
+| Dsn.go:50:29:50:33 | dbDSN | Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:50:29:50:33 | dbDSN | This query depends on a $@. | Dsn.go:47:10:47:30 | call to FormValue | user-provided value |

go/ql/test/experimental/CWE-134/DsnInjection.qlref

@@ -0,0 +1 @@
+experimental/CWE-134/DsnInjection.ql

go/ql/test/experimental/CWE-134/DsnInjectionLocal.expected

@@ -0,0 +1,27 @@
+edges
+| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:29:29:29:33 | dbDSN |
+| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:63:9:63:11 | cfg [pointer] |
+| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:67:102:67:104 | cfg [pointer] |
+| Dsn.go:63:9:63:11 | cfg [pointer] | Dsn.go:63:9:63:11 | implicit dereference |
+| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:62:2:62:4 | definition of cfg [pointer] |
+| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
+| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
+| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:9:63:11 | implicit dereference |
+| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:68:29:68:33 | dbDSN |
+| Dsn.go:67:102:67:104 | cfg [pointer] | Dsn.go:67:102:67:104 | implicit dereference |
+| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
+| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
+nodes
+| Dsn.go:26:11:26:17 | selection of Args | semmle.label | selection of Args |
+| Dsn.go:29:29:29:33 | dbDSN | semmle.label | dbDSN |
+| Dsn.go:62:2:62:4 | definition of cfg [pointer] | semmle.label | definition of cfg [pointer] |
+| Dsn.go:63:9:63:11 | cfg [pointer] | semmle.label | cfg [pointer] |
+| Dsn.go:63:9:63:11 | implicit dereference | semmle.label | implicit dereference |
+| Dsn.go:63:19:63:25 | selection of Args | semmle.label | selection of Args |
+| Dsn.go:67:102:67:104 | cfg [pointer] | semmle.label | cfg [pointer] |
+| Dsn.go:67:102:67:104 | implicit dereference | semmle.label | implicit dereference |
+| Dsn.go:68:29:68:33 | dbDSN | semmle.label | dbDSN |
+subpaths
+#select
+| Dsn.go:29:29:29:33 | dbDSN | Dsn.go:26:11:26:17 | selection of Args | Dsn.go:29:29:29:33 | dbDSN | This query depends on a $@. | Dsn.go:26:11:26:17 | selection of Args | user-provided value |
+| Dsn.go:68:29:68:33 | dbDSN | Dsn.go:63:19:63:25 | selection of Args | Dsn.go:68:29:68:33 | dbDSN | This query depends on a $@. | Dsn.go:63:19:63:25 | selection of Args | user-provided value |

go/ql/test/experimental/CWE-134/DsnInjectionLocal.qlref

@@ -0,0 +1 @@
+experimental/CWE-134/DsnInjectionLocal.ql

go/ql/test/experimental/CWE-203/Timing.expected

@@ -0,0 +1,10 @@
+edges
+| timing.go:14:18:14:27 | selection of Header | timing.go:14:18:14:45 | call to Get |
+| timing.go:14:18:14:45 | call to Get | timing.go:16:25:16:36 | headerSecret |
+nodes
+| timing.go:14:18:14:27 | selection of Header | semmle.label | selection of Header |
+| timing.go:14:18:14:45 | call to Get | semmle.label | call to Get |
+| timing.go:16:25:16:36 | headerSecret | semmle.label | headerSecret |
+subpaths
+#select
+| timing.go:16:25:16:36 | headerSecret | timing.go:14:18:14:27 | selection of Header | timing.go:16:25:16:36 | headerSecret | $@ may be vulnerable to timing attacks. | timing.go:14:18:14:27 | selection of Header | Hardcoded String |

go/ql/test/experimental/CWE-203/Timing.qlref

@@ -0,0 +1 @@
+experimental/CWE-203/Timing.ql

go/ql/test/experimental/CWE-203/timing.go

@@ -0,0 +1,37 @@
+package main
+
+import (
+	"crypto/subtle"
+	"fmt"
+	"net/http"
+)
+
+func bad(w http.ResponseWriter, req *http.Request) (interface{}, error) {
+
+	secret := "MySuperSecretPasscode"
+	secretHeader := "X-Secret"
+
+	headerSecret := req.Header.Get(secretHeader)
+	secretStr := string(secret)
+	if len(secret) != 0 && headerSecret != secretStr {
+		return nil, fmt.Errorf("header %s=%s did not match expected secret", secretHeader, headerSecret)
+	}
+	return nil, nil
+}
+
+func good(w http.ResponseWriter, req *http.Request) (interface{}, error) {
+
+	secret := []byte("MySuperSecretPasscode")
+	secretHeader := "X-Secret"
+
+	headerSecret := req.Header.Get(secretHeader)
+	if len(secret) != 0 && subtle.ConstantTimeCompare(secret, []byte(headerSecret)) != 1 {
+		return nil, fmt.Errorf("header %s=%s did not match expected secret", secretHeader, headerSecret)
+	}
+	return nil, nil
+}
+
+func main() {
+	bad(nil, nil)
+	good(nil, nil)
+}

java/documentation/library-coverage/coverage.csv

@@ -92,6 +92,7 @@ org.apache.commons.net,9,12,,,,,,,,,,,,,,,,6,,3,,,,,,,,,,,,,,,,,,,12,,
 org.apache.commons.ognl,6,,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,
 org.apache.commons.text,,,272,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,220,52
 org.apache.directory.ldap.client.api,1,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,
+org.apache.hadoop.fs,,,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,
 org.apache.hadoop.hive.metastore,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,,,,,,,,,,,
 org.apache.hc.client5.http.async.methods,84,,,,,,,,,,,,,,,,,84,,,,,,,,,,,,,,,,,,,,,,,
 org.apache.hc.client5.http.classic.methods,37,,,,,,,,,,,,,,,,,37,,,,,,,,,,,,,,,,,,,,,,,
@@ -133,7 +134,7 @@ org.springframework.cache,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13
 org.springframework.context,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
 org.springframework.data.repository,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1
 org.springframework.http,14,,71,,,,,,,,,,,,,,,14,,,,,,,,,,,,,,,,,,,,,,61,10
-org.springframework.jdbc.core,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,,,,,,,,,,,,
+org.springframework.jdbc.core,19,,,,,,,,,,,,,,,,,,,,,,,,,,,,19,,,,,,,,,,,,
 org.springframework.jdbc.datasource,4,,,,,,,,,,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
 org.springframework.jdbc.object,9,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,,
 org.springframework.jndi,1,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,

java/documentation/library-coverage/coverage.rst

@@ -21,7 +21,7 @@ Java framework & library support
    Java Standard Library,``java.*``,3,679,168,39,,9,,,13
    Java extensions,"``javax.*``, ``jakarta.*``",63,611,34,1,4,,1,1,2
    Kotlin Standard Library,``kotlin*``,,1843,16,11,,,,,2
-   `Spring <https://spring.io/>`_,``org.springframework.*``,29,483,104,2,,19,14,,29
-   Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.hubspot.jinjava``, ``com.mitchellbosecke.pebble``, ``com.opensymphony.xwork2.ognl``, ``com.rabbitmq.client``, ``com.thoughtworks.xstream``, ``com.unboundid.ldap.sdk``, ``com.zaxxer.hikari``, ``flexjson``, ``freemarker.cache``, ``freemarker.template``, ``groovy.lang``, ``groovy.util``, ``hudson``, ``io.jsonwebtoken``, ``io.netty.bootstrap``, ``io.netty.buffer``, ``io.netty.channel``, ``io.netty.handler.codec``, ``io.netty.handler.ssl``, ``io.netty.handler.stream``, ``io.netty.resolver``, ``io.netty.util``, ``javafx.scene.web``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``okhttp3``, ``org.apache.commons.codec``, ``org.apache.commons.compress.archivers.tar``, ``org.apache.commons.httpclient.util``, ``org.apache.commons.jelly``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.logging``, ``org.apache.commons.net``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.hadoop.hive.metastore``, ``org.apache.hc.client5.http.async.methods``, ``org.apache.hc.client5.http.classic.methods``, ``org.apache.hc.client5.http.fluent``, ``org.apache.hive.hcatalog.templeton``, ``org.apache.ibatis.jdbc``, ``org.apache.log4j``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.apache.tools.ant``, ``org.apache.tools.zip``, ``org.apache.velocity.app``, ``org.apache.velocity.runtime``, ``org.codehaus.cargo.container.installer``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.eclipse.jetty.client``, ``org.geogebra.web.full.main``, ``org.hibernate``, ``org.jdbi.v3.core``, ``org.jooq``, ``org.kohsuke.stapler``, ``org.mvel2``, ``org.openjdk.jmh.runner.options``, ``org.scijava.log``, ``org.slf4j``, ``org.thymeleaf``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``, ``retrofit2``",89,817,515,26,,18,18,,181
-   Totals,,246,9109,1957,174,10,113,33,1,361
+   `Spring <https://spring.io/>`_,``org.springframework.*``,29,483,113,2,,28,14,,29
+   Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.hubspot.jinjava``, ``com.mitchellbosecke.pebble``, ``com.opensymphony.xwork2.ognl``, ``com.rabbitmq.client``, ``com.thoughtworks.xstream``, ``com.unboundid.ldap.sdk``, ``com.zaxxer.hikari``, ``flexjson``, ``freemarker.cache``, ``freemarker.template``, ``groovy.lang``, ``groovy.util``, ``hudson``, ``io.jsonwebtoken``, ``io.netty.bootstrap``, ``io.netty.buffer``, ``io.netty.channel``, ``io.netty.handler.codec``, ``io.netty.handler.ssl``, ``io.netty.handler.stream``, ``io.netty.resolver``, ``io.netty.util``, ``javafx.scene.web``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``okhttp3``, ``org.apache.commons.codec``, ``org.apache.commons.compress.archivers.tar``, ``org.apache.commons.httpclient.util``, ``org.apache.commons.jelly``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.logging``, ``org.apache.commons.net``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.hadoop.fs``, ``org.apache.hadoop.hive.metastore``, ``org.apache.hc.client5.http.async.methods``, ``org.apache.hc.client5.http.classic.methods``, ``org.apache.hc.client5.http.fluent``, ``org.apache.hive.hcatalog.templeton``, ``org.apache.ibatis.jdbc``, ``org.apache.log4j``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.apache.tools.ant``, ``org.apache.tools.zip``, ``org.apache.velocity.app``, ``org.apache.velocity.runtime``, ``org.codehaus.cargo.container.installer``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.eclipse.jetty.client``, ``org.geogebra.web.full.main``, ``org.hibernate``, ``org.jdbi.v3.core``, ``org.jooq``, ``org.kohsuke.stapler``, ``org.mvel2``, ``org.openjdk.jmh.runner.options``, ``org.scijava.log``, ``org.slf4j``, ``org.thymeleaf``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``, ``retrofit2``",89,827,515,26,,18,18,,181
+   Totals,,246,9119,1966,174,10,122,33,1,361
 

java/kotlin-extractor/src/main/java/com/semmle/extractor/java/OdasaOutput.java

@@ -50,13 +50,9 @@ import com.semmle.util.trap.dependencies.TrapSet;
 import com.semmle.util.trap.pathtransformers.PathTransformer;
 
 public class OdasaOutput {
-	// either these are set ...
 	private final File trapFolder;
 	private final File sourceArchiveFolder;
 
-	// ... or this one is set
-	private final PopulationSpecFile specFile;
-
 	private File currentSourceFile;
 	private TrapSet trapsCreated;
 	private TrapDependencies trapDependenciesForSource;
@@ -72,29 +68,21 @@ public class OdasaOutput {
 	OdasaOutput(File outputRoot, Logger log) {
 		this.trapFolder = new File(outputRoot, "trap");
 		this.sourceArchiveFolder = new File(outputRoot, "src_archive");
-		this.specFile = null;
 		this.trackClassOrigins = false;
 		this.log = log;
 	}
 
 	public OdasaOutput(boolean trackClassOrigins, Logger log) {
 		String trapFolderVar = Env.systemEnv().getFirstNonEmpty("CODEQL_EXTRACTOR_JAVA_TRAP_DIR", Var.TRAP_FOLDER.name());
-		if (trapFolderVar != null) {
-			String sourceArchiveVar = Env.systemEnv().getFirstNonEmpty("CODEQL_EXTRACTOR_JAVA_SOURCE_ARCHIVE_DIR", Var.SOURCE_ARCHIVE.name());
-			if (sourceArchiveVar == null)
-				throw new ResourceError(Var.TRAP_FOLDER + " was set to '" + trapFolderVar + "', but "
-						+ Var.SOURCE_ARCHIVE + " was not set");
-			this.trapFolder = new File(trapFolderVar);
-			this.sourceArchiveFolder = new File(sourceArchiveVar);
-			this.specFile = null;
-		} else {
-			this.trapFolder = null;
-			this.sourceArchiveFolder = null;
-			String specFileVar = Env.systemEnv().get(Var.ODASA_JAVA_LAYOUT);
-			if (specFileVar == null)
-				throw new ResourceError("Neither " + Var.TRAP_FOLDER + " nor " + Var.ODASA_JAVA_LAYOUT + " was set");
-			this.specFile = new PopulationSpecFile(new File(specFileVar));
+		if (trapFolderVar == null) {
+			throw new ResourceError("CODEQL_EXTRACTOR_JAVA_TRAP_DIR was not set");
 		}
+		String sourceArchiveVar = Env.systemEnv().getFirstNonEmpty("CODEQL_EXTRACTOR_JAVA_SOURCE_ARCHIVE_DIR", Var.SOURCE_ARCHIVE.name());
+		if (sourceArchiveVar == null) {
+			throw new ResourceError("CODEQL_EXTRACTOR_JAVA_SOURCE_ARCHIVE_DIR was not set");
+    }
+		this.trapFolder = new File(trapFolderVar);
+		this.sourceArchiveFolder = new File(sourceArchiveVar);
 		this.trackClassOrigins = trackClassOrigins;
 		this.log = log;
 	}
@@ -123,11 +111,8 @@ public class OdasaOutput {
 
 	/** The output paths for that file, or null if it shouldn't be included */
 	private SpecFileEntry entryFor() {
-		if (specFile != null)
-			return specFile.getEntryFor(currentSourceFile);
-		else
-			return new SpecFileEntry(trapFolder, sourceArchiveFolder,
-					Arrays.asList(PathTransformer.std().fileAsDatabaseString(currentSourceFile)));
+		return new SpecFileEntry(trapFolder, sourceArchiveFolder,
+				Arrays.asList(PathTransformer.std().fileAsDatabaseString(currentSourceFile)));
 	}
 
 	/*

java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt

@@ -239,8 +239,6 @@ open class KotlinUsesExtractor(
         return UseClassInstanceResult(classTypeResult, extractClass)
     }
 
-    private fun isArray(t: IrSimpleType) = t.isBoxedArray || t.isPrimitiveArray()
-
     private fun extractClassLaterIfExternal(c: IrClass) {
         if (isExternalDeclaration(c)) {
             extractExternalClassLater(c)
@@ -551,6 +549,22 @@ open class KotlinUsesExtractor(
                 )
         }
 
+    /*
+    Kotlin arrays can be broken down as:
+
+    isArray(t)
+    |- t.isBoxedArray
+    |  |- t.isArray()         e.g. Array<Boolean>, Array<Boolean?>
+    |  |- t.isNullableArray() e.g. Array<Boolean>?, Array<Boolean?>?
+    |- t.isPrimitiveArray()   e.g. BooleanArray
+
+    For the corresponding Java types:
+    Boxed arrays are represented as e.g. java.lang.Boolean[].
+    Primitive arrays are represented as e.g. boolean[].
+    */
+
+    private fun isArray(t: IrType) = t.isBoxedArray || t.isPrimitiveArray()
+
     data class ArrayInfo(val elementTypeResults: TypeResults,
                          val componentTypeResults: TypeResults,
                          val dimensions: Int)
@@ -565,7 +579,7 @@ open class KotlinUsesExtractor(
      */
     private fun useArrayType(t: IrType, isPrimitiveArray: Boolean): ArrayInfo {
 
-        if (!t.isBoxedArray && !t.isPrimitiveArray()) {
+        if (!isArray(t)) {
             val nullableT = if (t.isPrimitiveType() && !isPrimitiveArray) t.makeNullable() else t
             val typeResults = useType(nullableT)
             return ArrayInfo(typeResults, typeResults, 0)
@@ -1141,13 +1155,13 @@ open class KotlinUsesExtractor(
                     }
                 } else {
                     t.classOrNull?.let { tCls ->
-                        if (t.isArray() || t.isNullableArray()) {
+                        if (t.isBoxedArray) {
                             (t.arguments.singleOrNull() as? IrTypeProjection)?.let { elementTypeArg ->
                                 val elementType = elementTypeArg.type
                                 val replacedElementType = kClassToJavaClass(elementType)
                                 if (replacedElementType !== elementType) {
                                     val newArg = makeTypeProjection(replacedElementType, elementTypeArg.variance)
-                                    return tCls.typeWithArguments(listOf(newArg)).codeQlWithHasQuestionMark(t.isNullableArray())
+                                    return tCls.typeWithArguments(listOf(newArg)).codeQlWithHasQuestionMark(t.isNullable())
                                 }
                             }
                         }
@@ -1578,7 +1592,7 @@ open class KotlinUsesExtractor(
             }
 
             if (owner is IrClass) {
-                if (t.isArray() || t.isNullableArray()) {
+                if (t.isBoxedArray) {
                     val elementType = t.getArrayElementType(pluginContext.irBuiltIns)
                     val erasedElementType = erase(elementType)
                     return owner.typeWith(erasedElementType).codeQlWithHasQuestionMark(t.isNullable())

java/ql/integration-tests/all-platforms/java/diagnostics/java-version-too-old/test.py

@@ -8,6 +8,17 @@ if "JAVA_HOME_8_X64" in os.environ:
   sep = ";" if platform.system() == "Windows" else ":"
   os.environ["PATH"] = "".join([os.path.join(os.environ["JAVA_HOME"], "bin"), sep, os.environ["PATH"]])
 
-run_codeql_database_create([], lang="java", runFunction = runUnsuccessfully, db = None)
+# Ensure the autobuilder *doesn't* see Java 11 or 17, which it could switch to in order to build the project:
+for k in ["JAVA_HOME_11_X64", "JAVA_HOME_17_X64"]:
+  if k in os.environ:
+    del os.environ[k]
+
+# Use a custom, empty toolchains.xml file so the autobuilder doesn't see any Java versions that may be
+# in a system-level toolchains file
+toolchains_path = os.path.join(os.getcwd(), 'toolchains.xml')
+
+run_codeql_database_create([], lang="java", runFunction = runUnsuccessfully, db = None, extra_env={
+  'LGTM_INDEX_MAVEN_TOOLCHAINS_FILE': toolchains_path
+})
 
 check_diagnostics()

java/ql/integration-tests/all-platforms/java/diagnostics/java-version-too-old/toolchains.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0"?>
+<toolchains xmlns="https://maven.apache.org/TOOLCHAINS/1.1.0"
+  xmlns:xsi="https://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="https://maven.apache.org/TOOLCHAINS/1.1.0 https://maven.apache.org/xsd/toolchains-1.1.0.xsd">
+</toolchains>

java/ql/lib/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ### Minor Analysis Improvements
 
+* Added SQL injection sinks for Spring JDBC's `NamedParameterJdbcOperations`.
+* Added models for the following packages:
+
+  * org.apache.hadoop.fs
 * Added the `ArithmeticCommon.qll` library to provide predicates for reasoning about arithmetic operations.
 * Added the `ArithmeticTaintedLocalQuery.qll` library to provide the `ArithmeticTaintedLocalOverflowFlow` and `ArithmeticTaintedLocalUnderflowFlow` taint-tracking modules to reason about arithmetic with unvalidated user input.
 * Added the `ArithmeticTaintedQuery.qll` library to provide the `RemoteUserInputOverflow` and `RemoteUserInputUnderflow` taint-tracking modules to reason about arithmetic with unvalidated user input.

java/ql/lib/change-notes/released/0.6.2.md

@@ -2,6 +2,10 @@
 
 ### Minor Analysis Improvements
 
+* Added SQL injection sinks for Spring JDBC's `NamedParameterJdbcOperations`.
+* Added models for the following packages:
+
+  * org.apache.hadoop.fs
 * Added the `ArithmeticCommon.qll` library to provide predicates for reasoning about arithmetic operations.
 * Added the `ArithmeticTaintedLocalQuery.qll` library to provide the `ArithmeticTaintedLocalOverflowFlow` and `ArithmeticTaintedLocalUnderflowFlow` taint-tracking modules to reason about arithmetic with unvalidated user input.
 * Added the `ArithmeticTaintedQuery.qll` library to provide the `RemoteUserInputOverflow` and `RemoteUserInputUnderflow` taint-tracking modules to reason about arithmetic with unvalidated user input.

java/ql/lib/ext/groovy.lang.model.yml

@@ -29,3 +29,4 @@ extensions:
       - ["groovy.lang", "GroovyShell", False, "run", "(String,String,String[])", "", "Argument[0]", "groovy", "manual"]
       - ["groovy.lang", "GroovyShell", False, "run", "(URI,List)", "", "Argument[0]", "groovy", "manual"]
       - ["groovy.lang", "GroovyShell", False, "run", "(URI,String[])", "", "Argument[0]", "groovy", "manual"]
+      - ["groovy.text", "TemplateEngine", True, "createTemplate", "", "", "Argument[0]", "groovy", "manual"]

java/ql/lib/ext/org.apache.hadoop.fs.model.yml

@@ -0,0 +1,15 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: summaryModel
+    data:
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(Path,Path)", "", "Argument[0]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(Path,Path)", "", "Argument[1]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(Path,String)", "", "Argument[0]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(Path,String)", "", "Argument[1]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(String,String,String)", "", "Argument[1]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(String,String,String)", "", "Argument[2]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(String,String)", "", "Argument[0]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(String,String)", "", "Argument[1]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(String)", "", "Argument[0]", "Argument[this]", "taint", "ai-manual"]
+      - ["org.apache.hadoop.fs", "Path", True, "Path", "(URI)", "", "Argument[0]", "Argument[this]", "taint", "ai-manual"]

java/ql/lib/ext/org.springframework.jdbc.core.namedparam.model.yml

@@ -0,0 +1,14 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "batchUpdate", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "execute", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "query", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "queryForList", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "queryForMap", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "queryForObject", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "queryForRowSet", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "queryForStream", "", "", "Argument[0]", "sql", "manual"]
+      - ["org.springframework.jdbc.core.namedparam", "NamedParameterJdbcOperations", True, "update", "", "", "Argument[0]", "sql", "manual"]

java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll

@@ -104,6 +104,17 @@ private predicate constantBooleanExpr(Expr e, boolean val) {
   CalcConstants::calculateBooleanValue(e) = val
 }
 
+pragma[nomagic]
+private predicate constantStringExpr(Expr e, string val) {
+  e.(CompileTimeConstantExpr).getStringValue() = val
+  or
+  exists(SsaExplicitUpdate v, Expr src |
+    e = v.getAUse() and
+    src = v.getDefiningExpr().(VariableAssign).getSource() and
+    constantStringExpr(src, val)
+  )
+}
+
 private boolean getBoolValue(Expr e) { constantBooleanExpr(e, result) }
 
 private int getIntValue(Expr e) { constantIntegerExpr(e, result) }
@@ -126,6 +137,14 @@ class ConstantBooleanExpr extends Expr {
   boolean getBooleanValue() { constantBooleanExpr(this, result) }
 }
 
+/** An expression that always has the same string value. */
+class ConstantStringExpr extends Expr {
+  ConstantStringExpr() { constantStringExpr(this, _) }
+
+  /** Get the string value of this expression. */
+  string getStringValue() { constantStringExpr(this, result) }
+}
+
 /**
  * Gets an expression that equals `v - d`.
  */

java/ql/lib/semmle/code/java/frameworks/Camel.qll

@@ -27,8 +27,8 @@ deprecated class CamelToURI = CamelToUri;
 class CamelToBeanUri extends CamelToUri {
   CamelToBeanUri() {
     // A `<to>` element references a bean if the URI starts with "bean:", or there is no scheme.
-    matches("bean:%") or
-    not exists(indexOf(":"))
+    this.matches("bean:%") or
+    not exists(this.indexOf(":"))
   }
 
   /**
@@ -38,13 +38,13 @@ class CamelToBeanUri extends CamelToUri {
    * parameter parts are optional.
    */
   string getBeanIdentifier() {
-    if not exists(indexOf(":"))
+    if not exists(this.indexOf(":"))
     then result = this
     else
-      exists(int start | start = indexOf(":", 0, 0) + 1 |
-        if not exists(indexOf("?"))
-        then result = suffix(start)
-        else result = substring(start, indexOf("?", 0, 0))
+      exists(int start | start = this.indexOf(":", 0, 0) + 1 |
+        if not exists(this.indexOf("?"))
+        then result = this.suffix(start)
+        else result = this.substring(start, this.indexOf("?", 0, 0))
       )
   }
 

java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll

@@ -0,0 +1,90 @@
+/** Provides XML definitions related to the `org.apache.commons` package. */
+
+import java
+private import semmle.code.java.dataflow.RangeUtils
+private import semmle.code.java.security.XmlParsers
+
+/**
+ * The classes `org.apache.commons.digester3.Digester`, `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`.
+ */
+private class Digester extends RefType {
+  Digester() {
+    this.hasQualifiedName([
+        "org.apache.commons.digester3", "org.apache.commons.digester",
+        "org.apache.tomcat.util.digester"
+      ], "Digester")
+  }
+}
+
+/** A call to `Digester.parse`. */
+private class DigesterParse extends XmlParserCall {
+  DigesterParse() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof Digester and
+      m.hasName("parse")
+    )
+  }
+
+  override Expr getSink() { result = this.getArgument(0) }
+
+  override predicate isSafe() { SafeDigesterFlow::flowToExpr(this.getQualifier()) }
+}
+
+/** A `ParserConfig` that is specific to `Digester`. */
+private class DigesterConfig extends ParserConfig {
+  DigesterConfig() {
+    exists(Method m |
+      m = this.getMethod() and
+      m.getDeclaringType() instanceof Digester and
+      m.hasName("setFeature")
+    )
+  }
+}
+
+/**
+ * A safely configured `Digester`.
+ */
+private class SafeDigester extends VarAccess {
+  SafeDigester() {
+    exists(Variable v | v = this.getVariable() |
+      exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
+        config.enables(singleSafeConfig())
+      )
+      or
+      exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
+        config
+            .disables(any(ConstantStringExpr s |
+                s.getStringValue() = "http://xml.org/sax/features/external-general-entities"
+              ))
+      ) and
+      exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
+        config
+            .disables(any(ConstantStringExpr s |
+                s.getStringValue() = "http://xml.org/sax/features/external-parameter-entities"
+              ))
+      ) and
+      exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
+        config
+            .disables(any(ConstantStringExpr s |
+                s.getStringValue() =
+                  "http://apache.org/xml/features/nonvalidating/load-external-dtd"
+              ))
+      )
+    )
+  }
+}
+
+private module SafeDigesterFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDigester }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof Digester
+    )
+  }
+
+  int fieldFlowBranchLimit() { result = 0 }
+}
+
+private module SafeDigesterFlow = DataFlow::Global<SafeDigesterFlowConfig>;

java/ql/lib/semmle/code/java/frameworks/javaee/Xml.qll

@@ -0,0 +1,64 @@
+/** Provides definitions related to the `javax.xml` package. */
+
+import java
+private import semmle.code.java.security.XmlParsers
+
+/** A call to `Validator.validate`. */
+private class ValidatorValidate extends XmlParserCall {
+  ValidatorValidate() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof Validator and
+      m.hasName("validate")
+    )
+  }
+
+  override Expr getSink() { result = this.getArgument(0) }
+
+  override predicate isSafe() { SafeValidatorFlow::flowToExpr(this.getQualifier()) }
+}
+
+/** A `TransformerConfig` specific to `Validator`. */
+private class ValidatorConfig extends TransformerConfig {
+  ValidatorConfig() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof Validator and
+      m.hasName("setProperty")
+    )
+  }
+}
+
+/** The class `javax.xml.validation.Validator`. */
+private class Validator extends RefType {
+  Validator() { this.hasQualifiedName("javax.xml.validation", "Validator") }
+}
+
+/** A safely configured `Validator`. */
+private class SafeValidator extends VarAccess {
+  SafeValidator() {
+    exists(Variable v | v = this.getVariable() |
+      exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() |
+        config.disables(configAccessExternalDtd())
+      ) and
+      exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() |
+        config.disables(configAccessExternalSchema())
+      )
+    )
+  }
+}
+
+private module SafeValidatorFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeValidator }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      sink.asExpr() = ma.getQualifier() and
+      ma.getMethod().getDeclaringType() instanceof Validator
+    )
+  }
+
+  int fieldFlowBranchLimit() { result = 0 }
+}
+
+private module SafeValidatorFlow = DataFlow::Global<SafeValidatorFlowConfig>;

java/ql/lib/semmle/code/java/frameworks/javase/Beans.qll

@@ -0,0 +1,24 @@
+/** Provides definitions related to the `java.beans` package. */
+
+import java
+private import semmle.code.java.security.XmlParsers
+
+/** The class `java.beans.XMLDecoder`. */
+private class XmlDecoder extends RefType {
+  XmlDecoder() { this.hasQualifiedName("java.beans", "XMLDecoder") }
+}
+
+/** A call to `XMLDecoder.readObject`. */
+private class XmlDecoderReadObject extends XmlParserCall {
+  XmlDecoderReadObject() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof XmlDecoder and
+      m.hasName("readObject")
+    )
+  }
+
+  override Expr getSink() { result = this.getQualifier() }
+
+  override predicate isSafe() { none() }
+}

java/ql/lib/semmle/code/java/frameworks/rundeck/RundeckXml.qll

@@ -0,0 +1,19 @@
+/** Provides definitions related to XML parsing in Rundeck. */
+
+import java
+private import semmle.code.java.security.XmlParsers
+
+/** A call to `ParserHelper.loadDocument`. */
+private class ParserHelperLoadDocument extends XmlParserCall {
+  ParserHelperLoadDocument() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType().hasQualifiedName("org.rundeck.api.parser", "ParserHelper") and
+      m.hasName("loadDocument")
+    )
+  }
+
+  override Expr getSink() { result = this.getArgument(0) }
+
+  override predicate isSafe() { none() }
+}