Merge pull request #14006 from MathiasVP/promote-invalid-pointer-deref-out-of-experimental

C++: Promote `cpp/invalid-pointer-deref` out of experimental

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

@@ -550,11 +550,14 @@ class SsaPhiNode extends Node, TSsaPhiNode {
    * `fromBackEdge` is true if data flows along a back-edge,
    * and `false` otherwise.
    */
+  cached
   final Node getAnInput(boolean fromBackEdge) {
     localFlowStep(result, this) and
-    if phi.getBasicBlock().dominates(result.getBasicBlock())
-    then fromBackEdge = true
-    else fromBackEdge = false
+    exists(IRBlock bPhi, IRBlock bResult |
+      bPhi = phi.getBasicBlock() and bResult = result.getBasicBlock()
+    |
+      if bPhi.dominates(bResult) then fromBackEdge = true else fromBackEdge = false
+    )
   }
 
   /** Gets a node that is used as input to this phi node. */

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

@@ -87,6 +87,30 @@ module ProductFlow {
      * dataflow graph.
      */
     default predicate isBarrierIn2(DataFlow::Node node) { none() }
+
+    /**
+     * Gets the virtual dispatch branching limit when calculating field flow in the first
+     * projection of the product dataflow graph.
+     *
+     * This can be overridden to a smaller value to improve performance (a
+     * value of 0 disables field flow), or a larger value to get more results.
+     */
+    default int fieldFlowBranchLimit1() {
+      // NOTE: This should be synchronized with the default value in the shared dataflow library
+      result = 2
+    }
+
+    /**
+     * Gets the virtual dispatch branching limit when calculating field flow in the second
+     * projection of the product dataflow graph.
+     *
+     * This can be overridden to a smaller value to improve performance (a
+     * value of 0 disables field flow), or a larger value to get more results.
+     */
+    default int fieldFlowBranchLimit2() {
+      // NOTE: This should be synchronized with the default value in the shared dataflow library
+      result = 2
+    }
   }
 
   /**
@@ -272,6 +296,30 @@ module ProductFlow {
      * dataflow graph.
      */
     default predicate isBarrierIn2(DataFlow::Node node) { none() }
+
+    /**
+     * Gets the virtual dispatch branching limit when calculating field flow in the first
+     * projection of the product dataflow graph.
+     *
+     * This can be overridden to a smaller value to improve performance (a
+     * value of 0 disables field flow), or a larger value to get more results.
+     */
+    default int fieldFlowBranchLimit1() {
+      // NOTE: This should be synchronized with the default value in the shared dataflow library
+      result = 2
+    }
+
+    /**
+     * Gets the virtual dispatch branching limit when calculating field flow in the second
+     * projection of the product dataflow graph.
+     *
+     * This can be overridden to a smaller value to improve performance (a
+     * value of 0 disables field flow), or a larger value to get more results.
+     */
+    default int fieldFlowBranchLimit2() {
+      // NOTE: This should be synchronized with the default value in the shared dataflow library
+      result = 2
+    }
   }
 
   /**
@@ -335,6 +383,8 @@ module ProductFlow {
       }
 
       predicate isBarrierIn(DataFlow::Node node) { Config::isBarrierIn1(node) }
+
+      int fieldFlowBranchLimit() { result = Config::fieldFlowBranchLimit1() }
     }
 
     private module Flow1 = DataFlow::GlobalWithState<Config1>;
@@ -367,6 +417,8 @@ module ProductFlow {
       }
 
       predicate isBarrierIn(DataFlow::Node node) { Config::isBarrierIn2(node) }
+
+      int fieldFlowBranchLimit() { result = Config::fieldFlowBranchLimit2() }
     }
 
     private module Flow2 = DataFlow::GlobalWithState<Config2>;

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/RangeAnalysisUtil.qll

@@ -1,6 +1,6 @@
 /**
- * This file contains the range-analysis specific parts of the `cpp/invalid-pointer-deref` query
- * that is used by both `AllocationToInvalidPointer.qll` and `InvalidPointerToDereference.qll`.
+ * This file contains the range-analysis specific parts of the `cpp/invalid-pointer-deref`
+ * and `cpp/overrun-write` query.
  */
 
 private import cpp

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticGuard.qll

@@ -39,6 +39,7 @@ predicate semImplies_v2(SemGuard g1, boolean b1, SemGuard g2, boolean b2) {
  * Holds if `guard` directly controls the position `controlled` with the
  * value `testIsTrue`.
  */
+pragma[nomagic]
 predicate semGuardDirectlyControlsSsaRead(
   SemGuard guard, SemSsaReadPosition controlled, boolean testIsTrue
 ) {

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/ModulusAnalysis.qll

@@ -17,19 +17,27 @@ private import RangeUtils
 private import RangeAnalysisStage
 
 module ModulusAnalysis<DeltaSig D, BoundSig<D> Bounds, UtilSig<D> U> {
-  /**
-   * Holds if `e + delta` equals `v` at `pos`.
-   */
-  private predicate valueFlowStepSsa(SemSsaVariable v, SemSsaReadPosition pos, SemExpr e, int delta) {
-    U::semSsaUpdateStep(v, e, D::fromInt(delta)) and pos.hasReadOfVar(v)
-    or
+  pragma[nomagic]
+  private predicate valueFlowStepSsaEqFlowCond(
+    SemSsaReadPosition pos, SemSsaVariable v, SemExpr e, int delta
+  ) {
     exists(SemGuard guard, boolean testIsTrue |
-      pos.hasReadOfVar(v) and
       guard = U::semEqFlowCond(v, e, D::fromInt(delta), true, testIsTrue) and
       semGuardDirectlyControlsSsaRead(guard, pos, testIsTrue)
     )
   }
 
+  /**
+   * Holds if `e + delta` equals `v` at `pos`.
+   */
+  pragma[nomagic]
+  private predicate valueFlowStepSsa(SemSsaVariable v, SemSsaReadPosition pos, SemExpr e, int delta) {
+    U::semSsaUpdateStep(v, e, D::fromInt(delta)) and pos.hasReadOfVar(v)
+    or
+    pos.hasReadOfVar(v) and
+    valueFlowStepSsaEqFlowCond(pos, v, e, delta)
+  }
+
   /**
    * Holds if `add` is the addition of `larg` and `rarg`, neither of which are
    * `ConstantIntegerExpr`s.

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeAnalysisStage.qll

@@ -660,7 +660,7 @@ module RangeStage<
    * - `upper = false` : `v >= b + delta`
    */
   private predicate boundedSsa(
-    SemSsaVariable v, SemSsaReadPosition pos, SemBound b, D::Delta delta, boolean upper,
+    SemSsaVariable v, SemBound b, D::Delta delta, SemSsaReadPosition pos, boolean upper,
     boolean fromBackEdge, D::Delta origdelta, SemReason reason
   ) {
     exists(SemExpr mid, D::Delta d1, D::Delta d2, SemReason r1, SemReason r2 |
@@ -673,10 +673,13 @@ module RangeStage<
     )
     or
     exists(D::Delta d, SemReason r1, SemReason r2 |
-      boundedSsa(v, pos, b, d, upper, fromBackEdge, origdelta, r2) or
-      boundedPhi(v, b, d, upper, fromBackEdge, origdelta, r2)
+      boundedSsa(pragma[only_bind_into](v), pragma[only_bind_into](b), pragma[only_bind_into](d),
+        pragma[only_bind_into](pos), upper, fromBackEdge, origdelta, r2)
+      or
+      boundedPhi(pragma[only_bind_into](v), pragma[only_bind_into](b), pragma[only_bind_into](d),
+        upper, fromBackEdge, origdelta, r2)
     |
-      unequalIntegralSsa(v, pos, b, d, r1) and
+      unequalIntegralSsa(v, b, d, pos, r1) and
       (
         upper = true and delta = D::fromFloat(D::toFloat(d) - 1)
         or
@@ -694,7 +697,7 @@ module RangeStage<
    * Holds if `v != b + delta` at `pos` and `v` is of integral type.
    */
   private predicate unequalIntegralSsa(
-    SemSsaVariable v, SemSsaReadPosition pos, SemBound b, D::Delta delta, SemReason reason
+    SemSsaVariable v, SemBound b, D::Delta delta, SemSsaReadPosition pos, SemReason reason
   ) {
     exists(SemExpr e, D::Delta d1, D::Delta d2 |
       unequalFlowStepIntegralSsa(v, pos, e, d1, reason) and
@@ -746,7 +749,7 @@ module RangeStage<
   ) {
     edge.phiInput(phi, inp) and
     exists(D::Delta d, boolean fromBackEdge0 |
-      boundedSsa(inp, edge, b, d, upper, fromBackEdge0, origdelta, reason)
+      boundedSsa(inp, b, d, edge, upper, fromBackEdge0, origdelta, reason)
       or
       boundedPhi(inp, b, d, upper, fromBackEdge0, origdelta, reason)
       or
@@ -1022,7 +1025,7 @@ module RangeStage<
       reason = TSemNoReason()
       or
       exists(SemSsaVariable v, SemSsaReadPositionBlock bb |
-        boundedSsa(v, bb, b, delta, upper, fromBackEdge, origdelta, reason) and
+        boundedSsa(v, b, delta, bb, upper, fromBackEdge, origdelta, reason) and
         e = v.getAUse() and
         bb.getBlock() = e.getBasicBlock()
       )

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/analysis/RangeUtils.qll

@@ -49,6 +49,7 @@ module RangeUtil<Range::DeltaSig D, Range::LangSig<D> Lang> implements Range::Ut
    * - `isEq = true`  : `v == e + delta`
    * - `isEq = false` : `v != e + delta`
    */
+  pragma[nomagic]
   SemGuard semEqFlowCond(
     SemSsaVariable v, SemExpr e, D::Delta delta, boolean isEq, boolean testIsTrue
   ) {

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/AllocationToInvalidPointer.qll

@@ -56,7 +56,7 @@ private import semmle.code.cpp.ir.dataflow.internal.ProductFlow
 private import semmle.code.cpp.ir.ValueNumbering
 private import semmle.code.cpp.controlflow.IRGuards
 private import codeql.util.Unit
-private import RangeAnalysisUtil
+private import semmle.code.cpp.rangeanalysis.new.RangeAnalysisUtil
 
 private VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
 
@@ -77,6 +77,15 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
   )
 }
 
+/**
+ * Gets the virtual dispatch branching limit when calculating field flow while searching
+ * for flow from an allocation to the construction of an out-of-bounds pointer.
+ *
+ * This can be overridden to a smaller value to improve performance (a
+ * value of 0 disables field flow), or a larger value to get more results.
+ */
+int allocationToInvalidPointerFieldFlowBranchLimit() { result = 0 }
+
 /**
  * A module that encapsulates a barrier guard to remove false positives from flow like:
  * ```cpp
@@ -101,9 +110,12 @@ private module SizeBarrier {
     predicate isSource(DataFlow::Node source) {
       // The sources is the same as in the sources for the second
       // projection in the `AllocToInvalidPointerConfig` module.
-      hasSize(_, source, _)
+      hasSize(_, source, _) and
+      InterestingPointerAddInstruction::isInterestingSize(source)
     }
 
+    int fieldFlowBranchLimit() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
+
     /**
      * Holds if `small <= large + k` holds if `g` evaluates to `testIsTrue`.
      */
@@ -201,6 +213,8 @@ private module InterestingPointerAddInstruction {
       hasSize(source.asConvertedExpr(), _, _)
     }
 
+    int fieldFlowBranchLimit() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
+
     predicate isSink(DataFlow::Node sink) {
       sink.asInstruction() = any(PointerAddInstruction pai).getLeft()
     }
@@ -220,6 +234,19 @@ private module InterestingPointerAddInstruction {
       flowTo(n)
     )
   }
+
+  /**
+   * Holds if `n` is a size of an allocation whose result flows to the left operand
+   * of a pointer-arithmetic instruction.
+   *
+   * This predicate is used to reduce the set of tuples in `SizeBarrierConfig::isSource`.
+   */
+  predicate isInterestingSize(DataFlow::Node n) {
+    exists(DataFlow::Node alloc |
+      hasSize(alloc.asConvertedExpr(), n, _) and
+      flow(alloc, _)
+    )
+  }
 }
 
 /**
@@ -244,6 +271,10 @@ private module Config implements ProductFlow::StateConfigSig {
     hasSize(allocSource.asConvertedExpr(), sizeSource, sizeAddend)
   }
 
+  int fieldFlowBranchLimit1() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
+
+  int fieldFlowBranchLimit2() { result = allocationToInvalidPointerFieldFlowBranchLimit() }
+
   predicate isSinkPair(
     DataFlow::Node allocSink, FlowState1 unit, DataFlow::Node sizeSink, FlowState2 sizeAddend
   ) {

cpp/ql/lib/semmle/code/cpp/security/InvalidPointerDereference/InvalidPointerToDereference.qll

@@ -81,7 +81,17 @@ private import semmle.code.cpp.dataflow.new.DataFlow
 private import semmle.code.cpp.ir.ValueNumbering
 private import semmle.code.cpp.controlflow.IRGuards
 private import AllocationToInvalidPointer as AllocToInvalidPointer
-private import RangeAnalysisUtil
+private import semmle.code.cpp.rangeanalysis.new.RangeAnalysisUtil
+
+/**
+ * Gets the virtual dispatch branching limit when calculating field flow while
+ * searching for flow from an out-of-bounds pointer to a dereference of the
+ * pointer.
+ *
+ * This can be overridden to a smaller value to improve performance (a
+ * value of 0 disables field flow), or a larger value to get more results.
+ */
+int invalidPointerToDereferenceFieldFlowBranchLimit() { result = 0 }
 
 private module InvalidPointerToDerefBarrier {
   private module BarrierConfig implements DataFlow::ConfigSig {
@@ -101,6 +111,8 @@ private module InvalidPointerToDerefBarrier {
     }
 
     predicate isSink(DataFlow::Node sink) { isSink(_, sink, _, _, _) }
+
+    int fieldFlowBranchLimit() { result = invalidPointerToDereferenceFieldFlowBranchLimit() }
   }
 
   private module BarrierFlow = DataFlow::Global<BarrierConfig>;
@@ -178,6 +190,8 @@ private module InvalidPointerToDerefConfig implements DataFlow::StateConfigSig {
     // Note that this is the only place where the `FlowState` is used in this configuration.
     node = InvalidPointerToDerefBarrier::getABarrierNode(pai)
   }
+
+  int fieldFlowBranchLimit() { result = invalidPointerToDereferenceFieldFlowBranchLimit() }
 }
 
 private import DataFlow::GlobalWithState<InvalidPointerToDerefConfig>

cpp/ql/src/Security/CWE/CWE-119/OverrunWriteProductFlow.ql

@@ -20,28 +20,10 @@ import semmle.code.cpp.models.interfaces.Allocation
 import semmle.code.cpp.models.interfaces.ArrayFunction
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysis
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
+import semmle.code.cpp.rangeanalysis.new.RangeAnalysisUtil
 import StringSizeFlow::PathGraph1
 import codeql.util.Unit
 
-pragma[nomagic]
-Instruction getABoundIn(SemBound b, IRFunction func) {
-  getSemanticExpr(result) = b.getExpr(0) and
-  result.getEnclosingIRFunction() = func
-}
-
-/**
- * Holds if `i <= b + delta`.
- */
-bindingset[i]
-pragma[inline_late]
-predicate bounded(Instruction i, Instruction b, int delta) {
-  exists(SemBound bound, IRFunction func |
-    semBounded(getSemanticExpr(i), bound, delta, true, _) and
-    b = getABoundIn(bound, func) and
-    i.getEnclosingIRFunction() = func
-  )
-}
-
 VariableAccess getAVariableAccess(Expr e) { e.getAChild*() = result }
 
 /**

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

@@ -3,7 +3,7 @@
   "qhelp.dtd">
 <qhelp>
 <overview>
-<p>The program performs an out-of-bounds read or write operation. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
+<p>The program performs an out-of-bounds read or write operation, which can cause program instability. In addition, attackers may take advantage of the situation, and implement techniques to use this vulnerability to execute arbitrary code.</p>
 
 </overview>
 <recommendation>
@@ -13,7 +13,7 @@
 </recommendation>
 <example>
 <p>The first example allocates a buffer of size <code>size</code> and creates a local variable that stores the location that is one byte past the end of the allocation.
-This local variable is then dereferenced which results in an out-of-bounds write.
+This local variable is then dereferenced, which results in an out-of-bounds write.
 The second example subtracts one from the <code>end</code> variable before dereferencing it. This subtraction ensures that the write correctly updates the final byte of the allocation.</p>
 <sample src="InvalidPointerDeref.cpp" />
 

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

@@ -1,10 +1,10 @@
 /**
  * @name Invalid pointer dereference
- * @description Dereferencing a pointer that points past it allocation is undefined behavior
- *              and may lead to security vulnerabilities.
+ * @description Dereferencing an out-of-bounds pointer is undefined behavior and may lead to security vulnerabilities.
  * @kind path-problem
  * @problem.severity error
- * @precision high
+ * @security-severity 9.3
+ * @precision medium
  * @id cpp/invalid-pointer-deref
  * @tags reliability
  *       security
@@ -94,6 +94,12 @@ module FinalConfig implements DataFlow::StateConfigSig {
     )
   }
 
+  int fieldFlowBranchLimit() {
+    result =
+      allocationToInvalidPointerFieldFlowBranchLimit()
+          .maximum(invalidPointerToDereferenceFieldFlowBranchLimit())
+  }
+
   predicate isAdditionalFlowStep(
     DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
   ) {

cpp/ql/src/change-notes/2023-08-21-invalid-pointer-deref.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `cpp/invalid-pointer-deref`, to detect out-of-bounds pointer reads and writes.

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

@@ -1 +0,0 @@
-experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

cpp/ql/test/query-tests/Security/CWE/CWE-193/AllocationToInvalidPointer.expected

@@ -1,2 +1,2 @@
-failures
 testFailures
+failures

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

@@ -45,58 +45,6 @@ edges
 | test.cpp:53:5:53:23 | ... = ... | test.cpp:51:33:51:35 | end |
 | test.cpp:53:12:53:23 | ... + ... | test.cpp:53:5:53:23 | ... = ... |
 | test.cpp:60:34:60:37 | mk_array output argument | test.cpp:67:9:67:14 | ... = ... |
-| test.cpp:80:9:80:16 | mk_array indirection [end] | test.cpp:89:19:89:26 | call to mk_array [end] |
-| test.cpp:80:9:80:16 | mk_array indirection [end] | test.cpp:119:18:119:25 | call to mk_array [end] |
-| test.cpp:82:5:82:28 | ... = ... | test.cpp:82:9:82:13 | arr indirection [post update] [begin] |
-| test.cpp:82:9:82:13 | arr indirection [post update] [begin] | test.cpp:83:15:83:17 | arr indirection [begin] |
-| test.cpp:82:17:82:22 | call to malloc | test.cpp:82:5:82:28 | ... = ... |
-| test.cpp:83:5:83:30 | ... = ... | test.cpp:83:9:83:11 | arr indirection [post update] [end] |
-| test.cpp:83:9:83:11 | arr indirection [post update] [end] | test.cpp:80:9:80:16 | mk_array indirection [end] |
-| test.cpp:83:15:83:17 | arr indirection [begin] | test.cpp:83:19:83:23 | begin indirection |
-| test.cpp:83:15:83:30 | ... + ... | test.cpp:83:5:83:30 | ... = ... |
-| test.cpp:83:19:83:23 | begin indirection | test.cpp:83:5:83:30 | ... = ... |
-| test.cpp:83:19:83:23 | begin indirection | test.cpp:83:15:83:30 | ... + ... |
-| test.cpp:89:19:89:26 | call to mk_array [end] | test.cpp:91:36:91:38 | arr indirection [end] |
-| test.cpp:89:19:89:26 | call to mk_array [end] | test.cpp:95:36:95:38 | arr indirection [end] |
-| test.cpp:91:36:91:38 | arr indirection [end] | test.cpp:91:40:91:42 | end indirection |
-| test.cpp:91:36:91:38 | arr indirection [end] | test.cpp:96:9:96:14 | ... = ... |
-| test.cpp:91:40:91:42 | end indirection | test.cpp:96:9:96:14 | ... = ... |
-| test.cpp:95:36:95:38 | arr indirection [end] | test.cpp:95:40:95:42 | end indirection |
-| test.cpp:95:36:95:38 | arr indirection [end] | test.cpp:96:9:96:14 | ... = ... |
-| test.cpp:95:40:95:42 | end indirection | test.cpp:96:9:96:14 | ... = ... |
-| test.cpp:104:27:104:29 | arr [end] | test.cpp:105:36:105:38 | arr indirection [end] |
-| test.cpp:104:27:104:29 | arr [end] | test.cpp:109:36:109:38 | arr indirection [end] |
-| test.cpp:105:36:105:38 | arr indirection [end] | test.cpp:105:40:105:42 | end indirection |
-| test.cpp:105:36:105:38 | arr indirection [end] | test.cpp:110:9:110:14 | ... = ... |
-| test.cpp:105:40:105:42 | end indirection | test.cpp:110:9:110:14 | ... = ... |
-| test.cpp:109:36:109:38 | arr indirection [end] | test.cpp:109:40:109:42 | end indirection |
-| test.cpp:109:36:109:38 | arr indirection [end] | test.cpp:110:9:110:14 | ... = ... |
-| test.cpp:109:40:109:42 | end indirection | test.cpp:110:9:110:14 | ... = ... |
-| test.cpp:119:18:119:25 | call to mk_array [end] | test.cpp:104:27:104:29 | arr [end] |
-| test.cpp:141:10:141:19 | mk_array_p indirection [end] | test.cpp:150:20:150:29 | call to mk_array_p indirection [end] |
-| test.cpp:141:10:141:19 | mk_array_p indirection [end] | test.cpp:180:19:180:28 | call to mk_array_p indirection [end] |
-| test.cpp:143:5:143:29 | ... = ... | test.cpp:143:10:143:14 | arr indirection [post update] [begin] |
-| test.cpp:143:10:143:14 | arr indirection [post update] [begin] | test.cpp:144:16:144:18 | arr indirection [begin] |
-| test.cpp:143:18:143:23 | call to malloc | test.cpp:143:5:143:29 | ... = ... |
-| test.cpp:144:5:144:32 | ... = ... | test.cpp:144:10:144:12 | arr indirection [post update] [end] |
-| test.cpp:144:10:144:12 | arr indirection [post update] [end] | test.cpp:141:10:141:19 | mk_array_p indirection [end] |
-| test.cpp:144:16:144:18 | arr indirection [begin] | test.cpp:144:21:144:25 | begin indirection |
-| test.cpp:144:16:144:32 | ... + ... | test.cpp:144:5:144:32 | ... = ... |
-| test.cpp:144:21:144:25 | begin indirection | test.cpp:144:5:144:32 | ... = ... |
-| test.cpp:144:21:144:25 | begin indirection | test.cpp:144:16:144:32 | ... + ... |
-| test.cpp:150:20:150:29 | call to mk_array_p indirection [end] | test.cpp:156:37:156:39 | arr indirection [end] |
-| test.cpp:156:37:156:39 | arr indirection [end] | test.cpp:156:42:156:44 | end indirection |
-| test.cpp:156:37:156:39 | arr indirection [end] | test.cpp:157:9:157:14 | ... = ... |
-| test.cpp:156:42:156:44 | end indirection | test.cpp:157:9:157:14 | ... = ... |
-| test.cpp:165:29:165:31 | arr indirection [end] | test.cpp:166:37:166:39 | arr indirection [end] |
-| test.cpp:165:29:165:31 | arr indirection [end] | test.cpp:170:37:170:39 | arr indirection [end] |
-| test.cpp:166:37:166:39 | arr indirection [end] | test.cpp:166:42:166:44 | end indirection |
-| test.cpp:166:37:166:39 | arr indirection [end] | test.cpp:171:9:171:14 | ... = ... |
-| test.cpp:166:42:166:44 | end indirection | test.cpp:171:9:171:14 | ... = ... |
-| test.cpp:170:37:170:39 | arr indirection [end] | test.cpp:170:42:170:44 | end indirection |
-| test.cpp:170:37:170:39 | arr indirection [end] | test.cpp:171:9:171:14 | ... = ... |
-| test.cpp:170:42:170:44 | end indirection | test.cpp:171:9:171:14 | ... = ... |
-| test.cpp:180:19:180:28 | call to mk_array_p indirection [end] | test.cpp:165:29:165:31 | arr indirection [end] |
 | test.cpp:194:23:194:28 | call to malloc | test.cpp:195:17:195:23 | ... + ... |
 | test.cpp:194:23:194:28 | call to malloc | test.cpp:195:17:195:23 | ... + ... |
 | test.cpp:194:23:194:28 | call to malloc | test.cpp:201:5:201:19 | ... = ... |
@@ -233,6 +181,18 @@ edges
 | test.cpp:754:18:754:31 | new[] | test.cpp:772:16:772:29 | access to array |
 | test.cpp:754:18:754:31 | new[] | test.cpp:772:16:772:29 | access to array |
 | test.cpp:781:14:781:27 | new[] | test.cpp:786:18:786:27 | access to array |
+| test.cpp:792:60:792:62 | end | test.cpp:800:40:800:43 | mk_array_no_field_flow output argument |
+| test.cpp:792:60:792:62 | end | test.cpp:832:40:832:43 | mk_array_no_field_flow output argument |
+| test.cpp:793:14:793:19 | call to malloc | test.cpp:794:5:794:24 | ... = ... |
+| test.cpp:793:14:793:19 | call to malloc | test.cpp:794:12:794:24 | ... + ... |
+| test.cpp:794:5:794:24 | ... = ... | test.cpp:792:60:792:62 | end |
+| test.cpp:794:12:794:24 | ... + ... | test.cpp:794:5:794:24 | ... = ... |
+| test.cpp:800:40:800:43 | mk_array_no_field_flow output argument | test.cpp:807:7:807:12 | ... = ... |
+| test.cpp:815:52:815:54 | end | test.cpp:815:52:815:54 | end |
+| test.cpp:815:52:815:54 | end | test.cpp:821:7:821:12 | ... = ... |
+| test.cpp:815:52:815:54 | end | test.cpp:821:7:821:12 | ... = ... |
+| test.cpp:832:40:832:43 | mk_array_no_field_flow output argument | test.cpp:833:37:833:39 | end |
+| test.cpp:833:37:833:39 | end | test.cpp:815:52:815:54 | end |
 nodes
 | test.cpp:4:15:4:20 | call to malloc | semmle.label | call to malloc |
 | test.cpp:5:15:5:22 | ... + ... | semmle.label | ... + ... |
@@ -260,48 +220,6 @@ nodes
 | test.cpp:53:12:53:23 | ... + ... | semmle.label | ... + ... |
 | test.cpp:60:34:60:37 | mk_array output argument | semmle.label | mk_array output argument |
 | test.cpp:67:9:67:14 | ... = ... | semmle.label | ... = ... |
-| test.cpp:80:9:80:16 | mk_array indirection [end] | semmle.label | mk_array indirection [end] |
-| test.cpp:82:5:82:28 | ... = ... | semmle.label | ... = ... |
-| test.cpp:82:9:82:13 | arr indirection [post update] [begin] | semmle.label | arr indirection [post update] [begin] |
-| test.cpp:82:17:82:22 | call to malloc | semmle.label | call to malloc |
-| test.cpp:83:5:83:30 | ... = ... | semmle.label | ... = ... |
-| test.cpp:83:9:83:11 | arr indirection [post update] [end] | semmle.label | arr indirection [post update] [end] |
-| test.cpp:83:15:83:17 | arr indirection [begin] | semmle.label | arr indirection [begin] |
-| test.cpp:83:15:83:30 | ... + ... | semmle.label | ... + ... |
-| test.cpp:83:19:83:23 | begin indirection | semmle.label | begin indirection |
-| test.cpp:89:19:89:26 | call to mk_array [end] | semmle.label | call to mk_array [end] |
-| test.cpp:91:36:91:38 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:91:40:91:42 | end indirection | semmle.label | end indirection |
-| test.cpp:95:36:95:38 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:95:40:95:42 | end indirection | semmle.label | end indirection |
-| test.cpp:96:9:96:14 | ... = ... | semmle.label | ... = ... |
-| test.cpp:104:27:104:29 | arr [end] | semmle.label | arr [end] |
-| test.cpp:105:36:105:38 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:105:40:105:42 | end indirection | semmle.label | end indirection |
-| test.cpp:109:36:109:38 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:109:40:109:42 | end indirection | semmle.label | end indirection |
-| test.cpp:110:9:110:14 | ... = ... | semmle.label | ... = ... |
-| test.cpp:119:18:119:25 | call to mk_array [end] | semmle.label | call to mk_array [end] |
-| test.cpp:141:10:141:19 | mk_array_p indirection [end] | semmle.label | mk_array_p indirection [end] |
-| test.cpp:143:5:143:29 | ... = ... | semmle.label | ... = ... |
-| test.cpp:143:10:143:14 | arr indirection [post update] [begin] | semmle.label | arr indirection [post update] [begin] |
-| test.cpp:143:18:143:23 | call to malloc | semmle.label | call to malloc |
-| test.cpp:144:5:144:32 | ... = ... | semmle.label | ... = ... |
-| test.cpp:144:10:144:12 | arr indirection [post update] [end] | semmle.label | arr indirection [post update] [end] |
-| test.cpp:144:16:144:18 | arr indirection [begin] | semmle.label | arr indirection [begin] |
-| test.cpp:144:16:144:32 | ... + ... | semmle.label | ... + ... |
-| test.cpp:144:21:144:25 | begin indirection | semmle.label | begin indirection |
-| test.cpp:150:20:150:29 | call to mk_array_p indirection [end] | semmle.label | call to mk_array_p indirection [end] |
-| test.cpp:156:37:156:39 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:156:42:156:44 | end indirection | semmle.label | end indirection |
-| test.cpp:157:9:157:14 | ... = ... | semmle.label | ... = ... |
-| test.cpp:165:29:165:31 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:166:37:166:39 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:166:42:166:44 | end indirection | semmle.label | end indirection |
-| test.cpp:170:37:170:39 | arr indirection [end] | semmle.label | arr indirection [end] |
-| test.cpp:170:42:170:44 | end indirection | semmle.label | end indirection |
-| test.cpp:171:9:171:14 | ... = ... | semmle.label | ... = ... |
-| test.cpp:180:19:180:28 | call to mk_array_p indirection [end] | semmle.label | call to mk_array_p indirection [end] |
 | test.cpp:194:23:194:28 | call to malloc | semmle.label | call to malloc |
 | test.cpp:195:17:195:23 | ... + ... | semmle.label | ... + ... |
 | test.cpp:195:17:195:23 | ... + ... | semmle.label | ... + ... |
@@ -394,6 +312,17 @@ nodes
 | test.cpp:772:16:772:29 | access to array | semmle.label | access to array |
 | test.cpp:781:14:781:27 | new[] | semmle.label | new[] |
 | test.cpp:786:18:786:27 | access to array | semmle.label | access to array |
+| test.cpp:792:60:792:62 | end | semmle.label | end |
+| test.cpp:793:14:793:19 | call to malloc | semmle.label | call to malloc |
+| test.cpp:794:5:794:24 | ... = ... | semmle.label | ... = ... |
+| test.cpp:794:12:794:24 | ... + ... | semmle.label | ... + ... |
+| test.cpp:800:40:800:43 | mk_array_no_field_flow output argument | semmle.label | mk_array_no_field_flow output argument |
+| test.cpp:807:7:807:12 | ... = ... | semmle.label | ... = ... |
+| test.cpp:815:52:815:54 | end | semmle.label | end |
+| test.cpp:815:52:815:54 | end | semmle.label | end |
+| test.cpp:821:7:821:12 | ... = ... | semmle.label | ... = ... |
+| test.cpp:832:40:832:43 | mk_array_no_field_flow output argument | semmle.label | mk_array_no_field_flow output argument |
+| test.cpp:833:37:833:39 | end | semmle.label | end |
 subpaths
 #select
 | test.cpp:6:14:6:15 | * ... | test.cpp:4:15:4:20 | call to malloc | test.cpp:6:14:6:15 | * ... | 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 |
@@ -404,10 +333,6 @@ subpaths
 | test.cpp:42:14:42:15 | * ... | test.cpp:40:15:40:20 | call to malloc | test.cpp:42:14:42:15 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:40:15:40:20 | call to malloc | call to malloc | test.cpp:41:20:41:27 | ... - ... | ... - ... |
 | test.cpp:44:14:44:21 | * ... | test.cpp:40:15:40:20 | call to malloc | test.cpp:44:14:44:21 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:40:15:40:20 | call to malloc | call to malloc | test.cpp:41:20:41:27 | ... - ... | ... - ... |
 | test.cpp:67:9:67:14 | ... = ... | test.cpp:52:19:52:24 | call to malloc | test.cpp:67:9:67:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:52:19:52:24 | call to malloc | call to malloc | test.cpp:53:20:53:23 | size | size |
-| test.cpp:96:9:96:14 | ... = ... | test.cpp:82:17:82:22 | call to malloc | test.cpp:96:9:96:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:82:17:82:22 | call to malloc | call to malloc | test.cpp:83:27:83:30 | size | size |
-| test.cpp:110:9:110:14 | ... = ... | test.cpp:82:17:82:22 | call to malloc | test.cpp:110:9:110:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:82:17:82:22 | call to malloc | call to malloc | test.cpp:83:27:83:30 | size | size |
-| test.cpp:157:9:157:14 | ... = ... | test.cpp:143:18:143:23 | call to malloc | test.cpp:157:9:157:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:143:18:143:23 | call to malloc | call to malloc | test.cpp:144:29:144:32 | size | size |
-| test.cpp:171:9:171:14 | ... = ... | test.cpp:143:18:143:23 | call to malloc | test.cpp:171:9:171:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:143:18:143:23 | call to malloc | call to malloc | test.cpp:144:29:144:32 | size | size |
 | test.cpp:201:5:201:19 | ... = ... | test.cpp:194:23:194:28 | call to malloc | test.cpp:201:5:201:19 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:194:23:194:28 | call to malloc | call to malloc | test.cpp:195:21:195:23 | len | len |
 | test.cpp:213:5:213:13 | ... = ... | test.cpp:205:23:205:28 | call to malloc | test.cpp:213:5:213:13 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:205:23:205:28 | call to malloc | call to malloc | test.cpp:206:21:206:23 | len | len |
 | test.cpp:232:3:232:20 | ... = ... | test.cpp:231:18:231:30 | new[] | test.cpp:232:3:232:20 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:231:18:231:30 | new[] | new[] | test.cpp:232:11:232:15 | index | index |
@@ -434,3 +359,5 @@ subpaths
 | test.cpp:772:16:772:29 | access to array | test.cpp:754:18:754:31 | new[] | test.cpp:772:16:772:29 | access to array | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:754:18:754:31 | new[] | new[] | test.cpp:767:22:767:28 | ... + ... | ... + ... |
 | test.cpp:772:16:772:29 | access to array | test.cpp:754:18:754:31 | new[] | test.cpp:772:16:772:29 | access to array | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:754:18:754:31 | new[] | new[] | test.cpp:772:22:772:28 | ... + ... | ... + ... |
 | test.cpp:786:18:786:27 | access to array | test.cpp:781:14:781:27 | new[] | test.cpp:786:18:786:27 | access to array | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:781:14:781:27 | new[] | new[] | test.cpp:786:20:786:26 | ... + ... | ... + ... |
+| test.cpp:807:7:807:12 | ... = ... | test.cpp:793:14:793:19 | call to malloc | test.cpp:807:7:807:12 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:793:14:793:19 | call to malloc | call to malloc | test.cpp:794:21:794:24 | size | size |
+| test.cpp:821:7:821:12 | ... = ... | test.cpp:793:14:793:19 | call to malloc | test.cpp:821:7:821:12 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:793:14:793:19 | call to malloc | call to malloc | test.cpp:794:21:794:24 | size | size |

cpp/ql/test/query-tests/Security/CWE/CWE-193/InvalidPointerDeref.qlref

@@ -0,0 +1 @@
+Security/CWE/CWE-193/InvalidPointerDeref.ql

cpp/ql/test/query-tests/Security/CWE/CWE-193/InvalidPointerToDereference.expected

@@ -1,2 +1,2 @@
-failures
 testFailures
+failures

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

@@ -80,7 +80,7 @@ struct array_t {
 array_t mk_array(int size) {
     array_t arr;
     arr.begin = malloc(size);
-    arr.end = arr.begin + size; // $ alloc=L82
+    arr.end = arr.begin + size; // $ MISSING: alloc=L82
 
     return arr;
 }
@@ -93,7 +93,7 @@ void test6(int size) {
     }
 
     for (char* p = arr.begin; p <= arr.end; ++p) {
-        *p = 0; // $ deref=L83->L91->L96 deref=L83->L95->L96 // BAD
+        *p = 0; // $ MISSING: deref=L83->L91->L96 deref=L83->L95->L96 // BAD [NOT DETECTED]
     }
 
     for (char* p = arr.begin; p < arr.end; ++p) {
@@ -107,7 +107,7 @@ void test7_callee(array_t arr) {
     }
 
     for (char* p = arr.begin; p <= arr.end; ++p) {
-        *p = 0; // $ deref=L83->L105->L110 deref=L83->L109->L110 // BAD
+        *p = 0; // $ MISSING: deref=L83->L105->L110 deref=L83->L109->L110 // BAD [NOT DETECTED]
     }
 
     for (char* p = arr.begin; p < arr.end; ++p) {
@@ -141,7 +141,7 @@ void test8(int size) {
 array_t *mk_array_p(int size) {
     array_t *arr = (array_t*) malloc(sizeof(array_t));
     arr->begin = malloc(size);
-    arr->end = arr->begin + size; // $ alloc=L143
+    arr->end = arr->begin + size; // $ MISSING: alloc=L143
 
     return arr;
 }
@@ -154,7 +154,7 @@ void test9(int size) {
     }
 
     for (char* p = arr->begin; p <= arr->end; ++p) {
-        *p = 0; // $ deref=L144->L156->L157 // BAD
+        *p = 0; // $ MISSING: deref=L144->L156->L157 // BAD [NOT DETECTED]
     }
 
     for (char* p = arr->begin; p < arr->end; ++p) {
@@ -168,7 +168,7 @@ void test10_callee(array_t *arr) {
     }
 
     for (char* p = arr->begin; p <= arr->end; ++p) {
-        *p = 0; // $ deref=L144->L166->L171 deref=L144->L170->L171 // BAD
+        *p = 0; // $ MISSING: deref=L144->L166->L171 deref=L144->L170->L171 // BAD [NOT DETECTED]
     }
 
     for (char* p = arr->begin; p < arr->end; ++p) {
@@ -787,4 +787,48 @@ void test38_simple(unsigned size, unsigned pos, unsigned numParams) {
       }
     }
   }
-}
+}
+
+void mk_array_no_field_flow(int size, char** begin, char** end) {
+    *begin = malloc(size);
+    *end = *begin + size; // $ alloc=L793
+}
+
+void test6_no_field_flow(int size) {
+  char* begin;
+  char* end;
+  mk_array_no_field_flow(size, &begin, &end);
+
+  for (char* p = begin; p != end; ++p) {
+      *p = 0; // GOOD
+  }
+
+  for (char* p = begin; p <= end; ++p) {
+      *p = 0; // $ deref=L794->L802->L807 deref=L794->L806->L807 // BAD
+  }
+
+  for (char* p = begin; p < end; ++p) {
+      *p = 0; // GOOD
+  }
+}
+
+void test7_callee_no_field_flow(char* begin, char* end) {
+  for (char* p = begin; p != end; ++p) {
+      *p = 0; // GOOD
+  }
+
+  for (char* p = begin; p <= end; ++p) {
+      *p = 0; // $ deref=L794->L815->L821 deref=L794->L816->L821 deref=L794->L820->L821 // BAD
+  }
+
+  for (char* p = begin; p < end; ++p) {
+      *p = 0; // GOOD
+  }
+}
+
+void test7_no_field_flow(int size) {
+  char* begin;
+  char* end;
+  mk_array_no_field_flow(size, &begin, &end);
+  test7_callee_no_field_flow(begin, end);
+}