C++: handle interprocedural flows

This currently copy-pastes some predicates from InvalidPointerDeref.ql. Those should be moved to a library file in a followup
2026-05-02 12:15:17 +02:00 · 2022-09-29 16:09:48 -04:00
parent 99d7512881
commit f17b563692
2 changed files with 100 additions and 20 deletions
--- a/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql
+++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql
@@ -4,21 +4,98 @@
 */

 import experimental.semmle.code.cpp.semantic.analysis.RangeAnalysis
-import experimental.semmle.code.cpp.rangeanalysis.Bound
+import experimental.semmle.code.cpp.semantic.SemanticBound
 import experimental.semmle.code.cpp.semantic.SemanticExprSpecific
 import semmle.code.cpp.ir.IR
 import experimental.semmle.code.cpp.ir.dataflow.DataFlow
+import experimental.semmle.code.cpp.ir.dataflow.DataFlow2
+
+pragma[nomagic]
+Instruction getABoundIn(SemBound b, IRFunction func) {
+  result = b.getExpr(0) and
+  result.getEnclosingIRFunction() = func
+}
+
+/**
+ * Holds if `i <= b + delta`.
+ */
+pragma[nomagic]
+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
+  )
+}
+
+class FieldAddressToPointerArithmeticConf extends DataFlow::Configuration {
+  FieldAddressToPointerArithmeticConf() { this = "FieldAddressToPointerArithmeticConf" }
+
+  override predicate isSource(DataFlow::Node source) { isFieldAddressSource(_, source) }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(PointerAddInstruction pai | pai.getLeft() = sink.asInstruction())
+  }
+}
+
+predicate isFieldAddressSource(Field f, DataFlow::Node source) {
+  source.asInstruction().(FieldAddressInstruction).getField() = f
+}
+
+/**
+ * Holds if `sink` is a sink for `InvalidPointerToDerefConf` and `i` is a `StoreInstruction` that
+ * writes to an address that non-strictly upper-bounds `sink`, or `i` is a `LoadInstruction` that
+ * reads from an address that non-strictly upper-bounds `sink`.
+ */
+predicate isInvalidPointerDerefSink(DataFlow::Node sink, Instruction i, string operation) {
+  exists(AddressOperand addr, int delta |
+    bounded(addr.getDef(), sink.asInstruction(), delta) and
+    delta >= 0 and
+    i.getAnOperand() = addr
+  |
+    i instanceof StoreInstruction and
+    operation = "write"
+    or
+    i instanceof LoadInstruction and
+    operation = "read"
+  )
+}
+
+predicate isConstantSizeOverflowSource(Field f, PointerAddInstruction pai, int delta) {
+  exists(
+    int size, int bound, SemZeroBound b, FieldAddressToPointerArithmeticConf conf,
+    DataFlow::Node source, DataFlow::InstructionNode sink
+  |
+    conf.hasFlow(source, sink) and
+    isFieldAddressSource(f, source) and
+    pai.getLeft() = sink.asInstruction() and
+    f.getUnspecifiedType().(ArrayType).getArraySize() = size and
+    semBounded(getSemanticExpr(pai.getRight()), b, bound, true, _) and
+    delta = bound - size and
+    delta >= 0 and
+    size != 0 and
+    size != 1
+  )
+}
+
+class PointerArithmeticToDerefConf extends DataFlow2::Configuration {
+  PointerArithmeticToDerefConf() { this = "PointerArithmeticToDerefConf" }
+
+  override predicate isSource(DataFlow::Node source) {
+    isConstantSizeOverflowSource(_, source.asInstruction(), _)
+  }
+
+  override predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink(sink, _, _) }
+}

 from
-  FieldAddressInstruction fai, PointerArithmeticInstruction pai, AddressOperand ao, ZeroBound b,
-  int delta, int size
+  Field f, DataFlow::Node source, DataFlow::Node sink,
+  Instruction deref,
+  PointerArithmeticToDerefConf conf, string operation, int delta
 where
-  size = fai.getField().getUnspecifiedType().(ArrayType).getArraySize() and
-  DataFlow::localInstructionFlow(fai, pai.getLeft()) and
-  DataFlow::localInstructionFlow(pai, ao.getAnyDef()) and
-  semBounded(getSemanticExpr(pai.getRight()), b, delta, true, _) and
-  delta >= size and
-  size != 0 and // sometimes 0 or 1 is used for a variable-size array
-  size != 1
-select pai, "This pointer may have an off-by-" + (delta - size + 1) + " error allowing it to overrun $@",
-  fai.getField(), fai.getField().toString()
+  conf.hasFlow(source, sink) and
+  isInvalidPointerDerefSink(sink, deref, operation) and
+  isConstantSizeOverflowSource(f, source.asInstruction(), delta)
+select source,
+  "This pointer arithmetic may have an off-by-" + (delta + 1) + " error allowing it to overrun $@ at this $@",
+  f, f.getName(), deref, operation
--- 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
@@ -1,8 +1,11 @@
-| test.cpp:35:5:35:22 | PointerAdd: access to array | This pointer may have an off-by-1 error allowing it to overrun $@ | test.cpp:15:9:15:11 | buf | buf |
-| test.cpp:36:5:36:24 | PointerAdd: access to array | This pointer may have an off-by-2 error allowing it to overrun $@ | test.cpp:15:9:15:11 | buf | buf |
-| test.cpp:43:9:43:19 | PointerAdd: access to array | This pointer may have an off-by-1 error allowing it to overrun $@ | test.cpp:15:9:15:11 | buf | buf |
-| test.cpp:49:5:49:22 | PointerAdd: access to array | This pointer may have an off-by-1 error allowing it to overrun $@ | test.cpp:19:9:19:11 | buf | buf |
-| test.cpp:50:5:50:24 | PointerAdd: access to array | This pointer may have an off-by-2 error allowing it to overrun $@ | test.cpp:19:9:19:11 | buf | buf |
-| test.cpp:57:9:57:19 | PointerAdd: access to array | This pointer may have an off-by-1 error allowing it to overrun $@ | test.cpp:19:9:19:11 | buf | buf |
-| test.cpp:61:9:61:19 | PointerAdd: access to array | This pointer may have an off-by-2 error allowing it to overrun $@ | test.cpp:19:9:19:11 | buf | buf |
-| test.cpp:77:27:77:44 | PointerAdd: access to array | This pointer may have an off-by-1 error allowing it to overrun $@ | test.cpp:15:9:15:11 | buf | buf |
+| test.cpp:26:5:26:15 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:5:9:5:11 | buf | buf | test.cpp:26:5:26:19 | Store: ... = ... | write |
+| test.cpp:30:5:30:15 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:10:9:10:11 | buf | buf | test.cpp:30:5:30:19 | Store: ... = ... | write |
+| test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write |
+| test.cpp:36:5:36:24 | 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:36:5:36:28 | Store: ... = ... | write |
+| test.cpp:43:9:43:19 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:15:9:15:11 | buf | buf | test.cpp:43:9:43:23 | Store: ... = ... | write |
+| test.cpp:49:5:49:22 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:19:9:19:11 | buf | buf | test.cpp:49:5:49:26 | Store: ... = ... | write |
+| test.cpp:50:5:50:24 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@ | test.cpp:19:9:19:11 | buf | buf | test.cpp:50:5:50:28 | Store: ... = ... | write |
+| test.cpp:57:9:57:19 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:19:9:19:11 | buf | buf | test.cpp:57:9:57:23 | Store: ... = ... | write |
+| test.cpp:61:9:61:19 | access to array | This pointer arithmetic may have an off-by-1 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 | access to array | This pointer arithmetic may have an off-by-0 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 | access to array | This pointer arithmetic may have an off-by-0 error allowing it to overrun $@ at this $@ | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |