Merge remote-tracking branch 'upstream/main' into clears-content

2026-04-26 01:05:15 +02:00 · 2023-06-27 13:45:33 +02:00
parent 2628552ef4 df61eaf59f
commit b1ae3a0a7b
73 changed files with 2633 additions and 169 deletions
--- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll
+++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll
@@ -70,6 +70,27 @@ predicate semBackEdge(SemSsaPhiNode phi, SemSsaVariable inp, SemSsaReadPositionP
  // Conservatively assume that every edge is a back edge if we don't have dominance information.
  (
    phi.getBasicBlock().bbDominates(edge.getOrigBlock()) or
+    irreducibleSccEdge(phi.getBasicBlock(), edge.getOrigBlock()) or
    not edge.getOrigBlock().hasDominanceInformation()
  )
 }
+
+/**
+ * Holds if the edge from b1 to b2 is part of a multiple-entry cycle in an irreducible control flow
+ * graph.
+ *
+ * An ireducible control flow graph is one where the usual dominance-based back edge detection does
+ * not work, because there is a cycle with multiple entry points, meaning there are
+ * mutually-reachable basic blocks where neither dominates the other. For such a graph, we first
+ * remove all detectable back-edges using the normal condition that the predecessor block is
+ * dominated by the successor block, then mark all edges in a cycle in the resulting graph as back
+ * edges.
+ */
+private predicate irreducibleSccEdge(SemBasicBlock b1, SemBasicBlock b2) {
+  trimmedEdge(b1, b2) and trimmedEdge+(b2, b1)
+}
+
+private predicate trimmedEdge(SemBasicBlock pred, SemBasicBlock succ) {
+  pred.getASuccessor() = succ and
+  not succ.bbDominates(pred)
+}
--- a/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql
+++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql
@@ -14,7 +14,7 @@ import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysi
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.ir.dataflow.DataFlow
-import FieldAddressToDerefFlow::PathGraph
+import ArrayAddressToDerefFlow::PathGraph

 pragma[nomagic]
 Instruction getABoundIn(SemBound b, IRFunction func) {
@@ -78,28 +78,45 @@ predicate isInvalidPointerDerefSink2(DataFlow::Node sink, Instruction i, string
  )
 }

+predicate arrayTypeCand(ArrayType arrayType) {
+  any(Variable v).getUnspecifiedType() = arrayType and
+  exists(arrayType.getArraySize())
+}
+
 pragma[nomagic]
 predicate arrayTypeHasSizes(ArrayType arr, int baseTypeSize, int arraySize) {
+  arrayTypeCand(arr) and
  arr.getBaseType().getSize() = baseTypeSize and
  arr.getArraySize() = arraySize
 }

-predicate pointerArithOverflow0(
-  PointerArithmeticInstruction pai, Field f, int size, int bound, int delta
-) {
-  not f.getNamespace() instanceof StdNamespace and
-  arrayTypeHasSizes(f.getUnspecifiedType(), pai.getElementSize(), size) and
-  semBounded(getSemanticExpr(pai.getRight()), any(SemZeroBound b), bound, true, _) and
-  delta = bound - size and
-  delta >= 0 and
-  size != 0 and
-  size != 1
+bindingset[pai]
+pragma[inline_late]
+predicate constantUpperBounded(PointerArithmeticInstruction pai, int delta) {
+  semBounded(getSemanticExpr(pai.getRight()), any(SemZeroBound b), delta, true, _)
+}
+
+bindingset[pai, size]
+predicate pointerArithOverflow0Impl(PointerArithmeticInstruction pai, int size, int delta) {
+  exists(int bound |
+    constantUpperBounded(pai, bound) and
+    delta = bound - size and
+    delta >= 0 and
+    size != 0 and
+    size != 1
+  )
+}
+
+pragma[nomagic]
+predicate pointerArithOverflow0(PointerArithmeticInstruction pai, int delta) {
+  exists(int size |
+    arrayTypeHasSizes(_, pai.getElementSize(), size) and
+    pointerArithOverflow0Impl(pai, size, delta)
+  )
 }

 module PointerArithmeticToDerefConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) {
-    pointerArithOverflow0(source.asInstruction(), _, _, _, _)
-  }
+  predicate isSource(DataFlow::Node source) { pointerArithOverflow0(source.asInstruction(), _) }

  predicate isBarrierIn(DataFlow::Node node) { isSource(node) }

@@ -110,25 +127,38 @@ module PointerArithmeticToDerefConfig implements DataFlow::ConfigSig {

 module PointerArithmeticToDerefFlow = DataFlow::Global<PointerArithmeticToDerefConfig>;

-predicate pointerArithOverflow(
-  PointerArithmeticInstruction pai, Field f, int size, int bound, int delta
-) {
-  pointerArithOverflow0(pai, f, size, bound, delta) and
+predicate pointerArithOverflow(PointerArithmeticInstruction pai, int delta) {
+  pointerArithOverflow0(pai, delta) and
  PointerArithmeticToDerefFlow::flow(DataFlow::instructionNode(pai), _)
 }

-module FieldAddressToDerefConfig implements DataFlow::StateConfigSig {
+bindingset[v]
+predicate finalPointerArithOverflow(Variable v, PointerArithmeticInstruction pai, int delta) {
+  exists(int size |
+    arrayTypeHasSizes(pragma[only_bind_out](v.getUnspecifiedType()), pai.getElementSize(), size) and
+    pointerArithOverflow0Impl(pai, size, delta)
+  )
+}
+
+predicate isSourceImpl(DataFlow::Node source, Variable v) {
+  (
+    source.asInstruction().(FieldAddressInstruction).getField() = v
+    or
+    source.asInstruction().(VariableAddressInstruction).getAstVariable() = v
+  ) and
+  arrayTypeCand(v.getUnspecifiedType())
+}
+
+module ArrayAddressToDerefConfig implements DataFlow::StateConfigSig {
  newtype FlowState =
-    additional TArray(Field f) { pointerArithOverflow(_, f, _, _, _) } or
+    additional TArray() or
    additional TOverflowArithmetic(PointerArithmeticInstruction pai) {
-      pointerArithOverflow(pai, _, _, _, _)
+      pointerArithOverflow(pai, _)
    }

  predicate isSource(DataFlow::Node source, FlowState state) {
-    exists(Field f |
-      source.asInstruction().(FieldAddressInstruction).getField() = f and
-      state = TArray(f)
-    )
+    isSourceImpl(source, _) and
+    state = TArray()
  }

  predicate isSink(DataFlow::Node sink, FlowState state) {
@@ -147,27 +177,27 @@ module FieldAddressToDerefConfig implements DataFlow::StateConfigSig {
  predicate isAdditionalFlowStep(
    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
  ) {
-    exists(PointerArithmeticInstruction pai, Field f |
-      state1 = TArray(f) and
+    exists(PointerArithmeticInstruction pai |
+      state1 = TArray() and
      state2 = TOverflowArithmetic(pai) and
      pai.getLeft() = node1.asInstruction() and
      node2.asInstruction() = pai and
-      pointerArithOverflow(pai, f, _, _, _)
+      pointerArithOverflow(pai, _)
    )
  }
 }

-module FieldAddressToDerefFlow = DataFlow::GlobalWithState<FieldAddressToDerefConfig>;
+module ArrayAddressToDerefFlow = DataFlow::GlobalWithState<ArrayAddressToDerefConfig>;

 from
-  Field f, FieldAddressToDerefFlow::PathNode source, PointerArithmeticInstruction pai,
-  FieldAddressToDerefFlow::PathNode sink, Instruction deref, string operation, int delta
+  Variable v, ArrayAddressToDerefFlow::PathNode source, PointerArithmeticInstruction pai,
+  ArrayAddressToDerefFlow::PathNode sink, Instruction deref, string operation, int delta
 where
-  FieldAddressToDerefFlow::flowPath(source, sink) and
+  ArrayAddressToDerefFlow::flowPath(pragma[only_bind_into](source), pragma[only_bind_into](sink)) and
  isInvalidPointerDerefSink2(sink.getNode(), deref, operation) and
-  source.getState() = FieldAddressToDerefConfig::TArray(f) and
-  sink.getState() = FieldAddressToDerefConfig::TOverflowArithmetic(pai) and
-  pointerArithOverflow(pai, f, _, _, delta)
+  pragma[only_bind_out](sink.getState()) = ArrayAddressToDerefConfig::TOverflowArithmetic(pai) and
+  isSourceImpl(source.getNode(), v) and
+  finalPointerArithOverflow(v, pai, delta)
 select pai, source, sink,
  "This pointer arithmetic may have an off-by-" + (delta + 1) +
-    " error allowing it to overrun $@ at this $@.", f, f.getName(), deref, operation
+    " error allowing it to overrun $@ at this $@.", v, v.getName(), deref, operation
--- a/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql
+++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql
@@ -19,6 +19,8 @@ import cpp
 import semmle.code.cpp.ir.dataflow.internal.ProductFlow
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysis
 import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
+import semmle.code.cpp.ir.ValueNumbering
+import semmle.code.cpp.controlflow.IRGuards
 import semmle.code.cpp.ir.IR
 import codeql.util.Unit

@@ -67,6 +69,86 @@ predicate hasSize(HeuristicAllocationExpr alloc, DataFlow::Node n, int state) {
  )
 }

+/**
+ * A module that encapsulates a barrier guard to remove false positives from flow like:
+ * ```cpp
+ * char *p = new char[size];
+ * // ...
+ * unsigned n = size;
+ * // ...
+ * if(n < size) {
+ *   use(*p[n]);
+ * }
+ * ```
+ * In this case, the sink pair identified by the product flow library (without any additional barriers)
+ * would be `(p, n)` (where `n` is the `n` in `p[n]`), because there exists a pointer-arithmetic
+ * instruction `pai` such that:
+ * 1. The left-hand of `pai` flows from the allocation, and
+ * 2. The right-hand of `pai` is non-strictly upper bounded by `n` (where `n` is the `n` in `p[n]`)
+ * but because there's a strict comparison that compares `n` against the size of the allocation this
+ * snippet is fine.
+ */
+module Barrier2 {
+  private class FlowState2 = AllocToInvalidPointerConfig::FlowState2;
+
+  private module BarrierConfig2 implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      // The sources is the same as in the sources for the second
+      // projection in the `AllocToInvalidPointerConfig` module.
+      hasSize(_, source, _)
+    }
+
+    additional predicate isSink(
+      DataFlow::Node left, DataFlow::Node right, IRGuardCondition g, FlowState2 state,
+      boolean testIsTrue
+    ) {
+      // The sink is any "large" side of a relational comparison.
+      g.comparesLt(left.asOperand(), right.asOperand(), state, true, testIsTrue)
+    }
+
+    predicate isSink(DataFlow::Node sink) { isSink(_, sink, _, _, _) }
+  }
+
+  private import DataFlow::Global<BarrierConfig2>
+
+  private FlowState2 getAFlowStateForNode(DataFlow::Node node) {
+    exists(DataFlow::Node source |
+      flow(source, node) and
+      hasSize(_, source, result)
+    )
+  }
+
+  private predicate operandGuardChecks(
+    IRGuardCondition g, Operand left, Operand right, FlowState2 state, boolean edge
+  ) {
+    exists(DataFlow::Node nLeft, DataFlow::Node nRight, FlowState2 state0 |
+      nRight.asOperand() = right and
+      nLeft.asOperand() = left and
+      BarrierConfig2::isSink(nLeft, nRight, g, state0, edge) and
+      state = getAFlowStateForNode(nRight) and
+      state0 <= state
+    )
+  }
+
+  Instruction getABarrierInstruction(FlowState2 state) {
+    exists(IRGuardCondition g, ValueNumber value, Operand use, boolean edge |
+      use = value.getAUse() and
+      operandGuardChecks(pragma[only_bind_into](g), pragma[only_bind_into](use), _,
+        pragma[only_bind_into](state), pragma[only_bind_into](edge)) and
+      result = value.getAnInstruction() and
+      g.controls(result.getBlock(), edge)
+    )
+  }
+
+  DataFlow::Node getABarrierNode(FlowState2 state) {
+    result.asOperand() = getABarrierInstruction(state).getAUse()
+  }
+
+  IRBlock getABarrierBlock(FlowState2 state) {
+    result.getAnInstruction() = getABarrierInstruction(state)
+  }
+}
+
 /**
 * A product-flow configuration for flow from an (allocation, size) pair to a
 * pointer-arithmetic operation that is non-strictly upper-bounded by `allocation + size`.
@@ -111,15 +193,14 @@ module AllocToInvalidPointerConfig implements ProductFlow::StateConfigSig {
    exists(state1) and
    // We check that the delta computed by the range analysis matches the
    // state value that we set in `isSourcePair`.
-    exists(int delta |
-      isSinkImpl(_, sink1, sink2, delta) and
-      state2 = delta
-    )
+    isSinkImpl(_, sink1, sink2, state2)
  }

  predicate isBarrier1(DataFlow::Node node, FlowState1 state) { none() }

-  predicate isBarrier2(DataFlow::Node node, FlowState2 state) { none() }
+  predicate isBarrier2(DataFlow::Node node, FlowState2 state) {
+    node = Barrier2::getABarrierNode(state)
+  }

  predicate isBarrierIn1(DataFlow::Node node) { isSourcePair(node, _, _, _) }

@@ -160,13 +241,40 @@ pragma[nomagic]
 predicate pointerAddInstructionHasBounds(
  PointerAddInstruction pai, DataFlow::Node sink1, DataFlow::Node sink2, int delta
 ) {
-  exists(Instruction right |
+  InterestingPointerAddInstruction::isInteresting(pragma[only_bind_into](pai)) and
+  exists(Instruction right, Instruction instr2 |
    pai.getRight() = right and
    pai.getLeft() = sink1.asInstruction() and
-    bounded1(right, sink2.asInstruction(), delta)
+    instr2 = sink2.asInstruction() and
+    bounded1(right, instr2, delta) and
+    not right = Barrier2::getABarrierInstruction(delta) and
+    not instr2 = Barrier2::getABarrierInstruction(delta)
  )
 }

+module InterestingPointerAddInstruction {
+  private module PointerAddInstructionConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
+      // The sources is the same as in the sources for the second
+      // projection in the `AllocToInvalidPointerConfig` module.
+      hasSize(source.asConvertedExpr(), _, _)
+    }
+
+    predicate isSink(DataFlow::Node sink) {
+      sink.asInstruction() = any(PointerAddInstruction pai).getLeft()
+    }
+  }
+
+  private import DataFlow::Global<PointerAddInstructionConfig>
+
+  predicate isInteresting(PointerAddInstruction pai) {
+    exists(DataFlow::Node n |
+      n.asInstruction() = pai.getLeft() and
+      flowTo(n)
+    )
+  }
+}
+
 /**
 * Holds if `pai` is non-strictly upper bounded by `sink2 + delta` and `sink1` is the
 * left operand of the pointer-arithmetic operation.
@@ -246,12 +354,21 @@ module InvalidPointerToDerefFlow = DataFlow::Global<InvalidPointerToDerefConfig>
 predicate invalidPointerToDerefSource(
  PointerArithmeticInstruction pai, DataFlow::Node source, int delta
 ) {
-  exists(AllocToInvalidPointerFlow::PathNode1 p, DataFlow::Node sink1 |
-    pragma[only_bind_out](p.getNode()) = sink1 and
-    AllocToInvalidPointerFlow::flowPath(_, _, pragma[only_bind_into](p), _) and
-    isSinkImpl(pai, sink1, _, _) and
+  exists(
+    AllocToInvalidPointerFlow::PathNode1 p1, AllocToInvalidPointerFlow::PathNode2 p2,
+    DataFlow::Node sink1, DataFlow::Node sink2, int delta0
+  |
+    pragma[only_bind_out](p1.getNode()) = sink1 and
+    pragma[only_bind_out](p2.getNode()) = sink2 and
+    AllocToInvalidPointerFlow::flowPath(_, _, pragma[only_bind_into](p1), pragma[only_bind_into](p2)) and
+    // Note that `delta` is not necessarily equal to `delta0`:
+    // `delta0` is the constant offset added to the size of the allocation, and
+    // delta is the constant difference between the pointer-arithmetic instruction
+    // and the instruction computing the address for which we will search for a dereference.
+    isSinkImpl(pai, sink1, sink2, delta0) and
    bounded2(source.asInstruction(), pai, delta) and
-    delta >= 0
+    delta >= 0 and
+    not source.getBasicBlock() = Barrier2::getABarrierBlock(delta0)
  )
 }

--- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected
@@ -11,6 +11,13 @@ edges
 | test.cpp:77:32:77:34 | buf | test.cpp:77:26:77:44 | & ... |
 | test.cpp:79:27:79:34 | buf | test.cpp:70:33:70:33 | p |
 | test.cpp:79:32:79:34 | buf | test.cpp:79:27:79:34 | buf |
+| test.cpp:85:34:85:36 | buf | test.cpp:87:5:87:31 | access to array |
+| test.cpp:85:34:85:36 | buf | test.cpp:88:5:88:27 | access to array |
+| test.cpp:128:9:128:11 | arr | test.cpp:128:9:128:14 | access to array |
+| test.cpp:134:25:134:27 | arr | test.cpp:136:9:136:16 | ... += ... |
+| test.cpp:136:9:136:16 | ... += ... | test.cpp:138:13:138:15 | arr |
+| test.cpp:143:18:143:21 | asdf | test.cpp:134:25:134:27 | arr |
+| test.cpp:143:18:143:21 | asdf | test.cpp:143:18:143:21 | asdf |
 nodes
 | test.cpp:35:5:35:22 | access to array | semmle.label | access to array |
 | test.cpp:35:10:35:12 | buf | semmle.label | buf |
@@ -33,6 +40,16 @@ nodes
 | test.cpp:77:32:77:34 | buf | semmle.label | buf |
 | test.cpp:79:27:79:34 | buf | semmle.label | buf |
 | test.cpp:79:32:79:34 | buf | semmle.label | buf |
+| test.cpp:85:34:85:36 | buf | semmle.label | buf |
+| test.cpp:87:5:87:31 | access to array | semmle.label | access to array |
+| test.cpp:88:5:88:27 | access to array | semmle.label | access to array |
+| test.cpp:128:9:128:11 | arr | semmle.label | arr |
+| test.cpp:128:9:128:14 | access to array | semmle.label | access to array |
+| test.cpp:134:25:134:27 | arr | semmle.label | arr |
+| test.cpp:136:9:136:16 | ... += ... | semmle.label | ... += ... |
+| test.cpp:138:13:138:15 | arr | semmle.label | arr |
+| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
+| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
 subpaths
 #select
 | test.cpp:35:5:35:22 | PointerAdd: access to array | test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write |
@@ -44,3 +61,5 @@ subpaths
 | test.cpp:61:9:61:19 | PointerAdd: access to array | test.cpp:61:14:61:16 | buf | test.cpp:61:9:61:19 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:61:9:61:23 | Store: ... = ... | write |
 | test.cpp:72:5:72:15 | PointerAdd: access to array | test.cpp:79:32:79:34 | buf | test.cpp:72:5:72:15 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:72:5:72:19 | Store: ... = ... | write |
 | test.cpp:77:27:77:44 | PointerAdd: access to array | test.cpp:77:32:77:34 | buf | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
+| test.cpp:128:9:128:14 | PointerAdd: access to array | test.cpp:128:9:128:11 | arr | test.cpp:128:9:128:14 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:125:11:125:13 | arr | arr | test.cpp:128:9:128:18 | Store: ... = ... | write |
+| test.cpp:136:9:136:16 | PointerAdd: ... += ... | test.cpp:143:18:143:21 | asdf | test.cpp:138:13:138:15 | arr | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:142:10:142:13 | asdf | asdf | test.cpp:138:12:138:15 | Load: * ... | read |
--- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp
@@ -85,7 +85,7 @@ void testCharIndex(BigArray *arr) {
    char *charBuf = (char*) arr->buf;

    charBuf[MAX_SIZE_BYTES - 1] = 0; // GOOD
-    charBuf[MAX_SIZE_BYTES] = 0; // BAD [FALSE NEGATIVE]
+    charBuf[MAX_SIZE_BYTES] = 0; // BAD
 }

 void testEqRefinement() {
@@ -120,3 +120,25 @@ void testEqRefinement2() {
        }
    }
 }
+
+void testStackAllocated() {
+    char *arr[MAX_SIZE];
+
+    for(int i = 0; i <= MAX_SIZE; i++) {
+        arr[i] = 0; // BAD
+    }
+}
+
+int strncmp(const char*, const char*, int);
+
+char testStrncmp2(char *arr) {
+    if(strncmp(arr, "<test>", 6) == 0) {
+        arr += 6;
+    }
+    return *arr; // GOOD [FALSE POSITIVE]
+}
+
+void testStrncmp1() {
+    char asdf[5];
+    testStrncmp2(asdf);
+}
--- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected
@@ -781,11 +781,24 @@ edges
 | test.cpp:407:3:407:5 | val indirection [xs] | test.cpp:407:7:407:8 | xs indirection |
 | test.cpp:407:7:407:8 | xs indirection | test.cpp:407:7:407:8 | xs |
 | test.cpp:417:16:417:33 | new[] | test.cpp:419:7:419:8 | xs |
-| test.cpp:419:7:419:8 | xs | test.cpp:419:7:419:11 | access to array |
-| test.cpp:419:7:419:11 | access to array | test.cpp:419:7:419:15 | Store: ... = ... |
 | test.cpp:427:14:427:27 | new[] | test.cpp:433:5:433:6 | xs |
-| test.cpp:433:5:433:6 | xs | test.cpp:433:5:433:17 | access to array |
-| test.cpp:433:5:433:17 | access to array | test.cpp:433:5:433:21 | Store: ... = ... |
+| test.cpp:439:14:439:27 | new[] | test.cpp:444:5:444:6 | xs |
+| test.cpp:450:14:450:27 | new[] | test.cpp:455:5:455:6 | xs |
+| test.cpp:455:5:455:6 | xs | test.cpp:455:5:455:15 | access to array |
+| test.cpp:455:5:455:15 | access to array | test.cpp:455:5:455:19 | Store: ... = ... |
+| test.cpp:461:14:461:27 | new[] | test.cpp:466:5:466:6 | xs |
+| test.cpp:466:5:466:6 | xs | test.cpp:466:5:466:15 | access to array |
+| test.cpp:466:5:466:15 | access to array | test.cpp:466:5:466:19 | Store: ... = ... |
+| test.cpp:472:14:472:27 | new[] | test.cpp:477:5:477:6 | xs |
+| test.cpp:483:14:483:27 | new[] | test.cpp:488:5:488:6 | xs |
+| test.cpp:494:14:494:31 | new[] | test.cpp:499:5:499:6 | xs |
+| test.cpp:505:14:505:31 | new[] | test.cpp:510:5:510:6 | xs |
+| test.cpp:516:14:516:31 | new[] | test.cpp:521:5:521:6 | xs |
+| test.cpp:527:14:527:31 | new[] | test.cpp:532:5:532:6 | xs |
+| test.cpp:538:14:538:31 | new[] | test.cpp:543:5:543:6 | xs |
+| test.cpp:549:14:549:31 | new[] | test.cpp:554:5:554:6 | xs |
+| test.cpp:554:5:554:6 | xs | test.cpp:554:5:554:15 | access to array |
+| test.cpp:554:5:554:15 | access to array | test.cpp:554:5:554:19 | Store: ... = ... |
 nodes
 | test.cpp:4:15:4:20 | call to malloc | semmle.label | call to malloc |
 | test.cpp:5:15:5:15 | p | semmle.label | p |
@@ -1147,12 +1160,36 @@ nodes
 | test.cpp:407:7:407:8 | xs indirection | semmle.label | xs indirection |
 | test.cpp:417:16:417:33 | new[] | semmle.label | new[] |
 | test.cpp:419:7:419:8 | xs | semmle.label | xs |
-| test.cpp:419:7:419:11 | access to array | semmle.label | access to array |
-| test.cpp:419:7:419:15 | Store: ... = ... | semmle.label | Store: ... = ... |
 | test.cpp:427:14:427:27 | new[] | semmle.label | new[] |
 | test.cpp:433:5:433:6 | xs | semmle.label | xs |
-| test.cpp:433:5:433:17 | access to array | semmle.label | access to array |
-| test.cpp:433:5:433:21 | Store: ... = ... | semmle.label | Store: ... = ... |
+| test.cpp:439:14:439:27 | new[] | semmle.label | new[] |
+| test.cpp:444:5:444:6 | xs | semmle.label | xs |
+| test.cpp:450:14:450:27 | new[] | semmle.label | new[] |
+| test.cpp:455:5:455:6 | xs | semmle.label | xs |
+| test.cpp:455:5:455:15 | access to array | semmle.label | access to array |
+| test.cpp:455:5:455:19 | Store: ... = ... | semmle.label | Store: ... = ... |
+| test.cpp:461:14:461:27 | new[] | semmle.label | new[] |
+| test.cpp:466:5:466:6 | xs | semmle.label | xs |
+| test.cpp:466:5:466:15 | access to array | semmle.label | access to array |
+| test.cpp:466:5:466:19 | Store: ... = ... | semmle.label | Store: ... = ... |
+| test.cpp:472:14:472:27 | new[] | semmle.label | new[] |
+| test.cpp:477:5:477:6 | xs | semmle.label | xs |
+| test.cpp:483:14:483:27 | new[] | semmle.label | new[] |
+| test.cpp:488:5:488:6 | xs | semmle.label | xs |
+| test.cpp:494:14:494:31 | new[] | semmle.label | new[] |
+| test.cpp:499:5:499:6 | xs | semmle.label | xs |
+| test.cpp:505:14:505:31 | new[] | semmle.label | new[] |
+| test.cpp:510:5:510:6 | xs | semmle.label | xs |
+| test.cpp:516:14:516:31 | new[] | semmle.label | new[] |
+| test.cpp:521:5:521:6 | xs | semmle.label | xs |
+| test.cpp:527:14:527:31 | new[] | semmle.label | new[] |
+| test.cpp:532:5:532:6 | xs | semmle.label | xs |
+| test.cpp:538:14:538:31 | new[] | semmle.label | new[] |
+| test.cpp:543:5:543:6 | xs | semmle.label | xs |
+| test.cpp:549:14:549:31 | new[] | semmle.label | new[] |
+| test.cpp:554:5:554:6 | xs | semmle.label | xs |
+| test.cpp:554:5:554:15 | access to array | semmle.label | access to array |
+| test.cpp:554:5:554:19 | Store: ... = ... | semmle.label | Store: ... = ... |
 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 |
@@ -1180,5 +1217,6 @@ subpaths
 | test.cpp:372:15:372:16 | Load: * ... | test.cpp:363:14:363:27 | new[] | test.cpp:372:15:372:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:363:14:363:27 | new[] | new[] | test.cpp:365:19:365:22 | size | size |
 | test.cpp:384:13:384:16 | Load: * ... | test.cpp:377:14:377:27 | new[] | test.cpp:384:13:384:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:377:14:377:27 | new[] | new[] | test.cpp:378:20:378:23 | size | size |
 | test.cpp:395:5:395:13 | Store: ... = ... | test.cpp:388:14:388:27 | new[] | test.cpp:395:5:395:13 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:388:14:388:27 | new[] | new[] | test.cpp:389:19:389:22 | size | size |
-| test.cpp:419:7:419:15 | Store: ... = ... | test.cpp:417:16:417:33 | new[] | test.cpp:419:7:419:15 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:417:16:417:33 | new[] | new[] | test.cpp:419:10:419:10 | i | i |
-| test.cpp:433:5:433:21 | Store: ... = ... | test.cpp:427:14:427:27 | new[] | test.cpp:433:5:433:21 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:427:14:427:27 | new[] | new[] | test.cpp:433:8:433:16 | ... ++ | ... ++ |
+| test.cpp:455:5:455:19 | Store: ... = ... | test.cpp:450:14:450:27 | new[] | test.cpp:455:5:455:19 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:450:14:450:27 | new[] | new[] | test.cpp:455:8:455:14 | src_pos | src_pos |
+| test.cpp:466:5:466:19 | Store: ... = ... | test.cpp:461:14:461:27 | new[] | test.cpp:466:5:466:19 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:461:14:461:27 | new[] | new[] | test.cpp:466:8:466:14 | src_pos | src_pos |
+| test.cpp:554:5:554:19 | Store: ... = ... | test.cpp:549:14:549:31 | new[] | test.cpp:554:5:554:19 | Store: ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:549:14:549:31 | new[] | new[] | test.cpp:554:8:554:14 | src_pos | src_pos |
--- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp
+++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp
@@ -416,7 +416,7 @@ void test30(int *size)
    new_size = tmp_size + 1;
    char *xs = new char[new_size];
    for (int i = 0; i < new_size; i++) {
-      xs[i] = 0;  // GOOD [FALSE POSITIVE]
+      xs[i] = 0;  // GOOD
    }
  }
  *size = new_size;
@@ -430,6 +430,127 @@ void test31(unsigned size, unsigned src_pos)
  }
  unsigned dst_pos = src_pos;
  if(dst_pos < size - 3) {
-    xs[dst_pos++] = 0; // GOOD [FALSE POSITIVE]
+    xs[dst_pos++] = 0; // GOOD
+  }
+}
+
+void test31_simple1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple2(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size + 1) {
+    xs[src_pos] = 0; // BAD
+  }
+}
+
+void test31_simple3(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos - 1 < size) {
+    xs[src_pos] = 0; // BAD
+  }
+}
+
+void test31_simple4(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size - 1) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple5(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos + 1 < size) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple1_plus1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size + 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple2_plus1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size + 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size + 1) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple3_plus1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size + 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos - 1 < size) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple4_plus1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size + 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size - 1) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple5_plus1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size + 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos + 1 < size) {
+    xs[src_pos] = 0; // GOOD
+  }
+}
+
+void test31_simple1_sub1(unsigned size, unsigned src_pos)
+{
+  char *xs = new char[size - 1];
+  if (src_pos > size) {
+    src_pos = size;
+  }
+  if(src_pos < size) {
+    xs[src_pos] = 0; // BAD
  }
 }
--- a/cpp/ql/test/library-tests/ir/range-analysis/test.cpp
+++ b/cpp/ql/test/library-tests/ir/range-analysis/test.cpp
@@ -70,3 +70,27 @@ int f4(int x) {
    }
  }
 }
+
+// No interesting ranges to check here - this irreducible CFG caused an infinite loop due to back edge detection
+void gotoLoop(bool b1, bool b2)
+{
+  int j;
+
+  if (b1)
+    return;
+
+  if (!b2)
+  {
+    for (j = 0; j < 10; ++j)
+    {
+     goto main_decode_loop;
+    }
+  }
+  else
+  {
+    for (j = 0; j < 10; ++j)
+    {
+      main_decode_loop:
+    }
+  }
+}