C++: Reuse even more DataFlow::Nodes

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

@@ -220,9 +220,10 @@ private module IndirectOperands {
     int indirectionIndex;
 
     IndirectOperandFromIRRepr() {
-      exists(Operand repr |
-        repr = Ssa::getIRRepresentationOfIndirectOperand(operand, indirectionIndex) and
-        nodeHasOperand(this, repr, indirectionIndex - 1)
+      exists(Operand repr, int indirectionIndexRepr |
+        Ssa::hasIRRepresentationOfIndirectOperand(operand, indirectionIndex, repr,
+          indirectionIndexRepr) and
+        nodeHasOperand(this, repr, indirectionIndexRepr)
       )
     }
 
@@ -262,9 +263,10 @@ private module IndirectInstructions {
     int indirectionIndex;
 
     IndirectInstructionFromIRRepr() {
-      exists(Instruction repr |
-        repr = Ssa::getIRRepresentationOfIndirectInstruction(instr, indirectionIndex) and
-        nodeHasInstruction(this, repr, indirectionIndex - 1)
+      exists(Instruction repr, int indirectionIndexRepr |
+        Ssa::hasIRRepresentationOfIndirectInstruction(instr, indirectionIndex, repr,
+          indirectionIndexRepr) and
+        nodeHasInstruction(this, repr, indirectionIndexRepr)
       )
     }
 
@@ -690,7 +692,7 @@ predicate storeStep(Node node1, ContentSet c, Node node2) { storeStepImpl(node1,
 private predicate numberOfLoadsFromOperandRec(
   Operand operandFrom, Operand operandTo, int ind, boolean certain
 ) {
-  exists(Instruction load | Ssa::isDereference(load, operandFrom) |
+  exists(Instruction load | Ssa::isDereference(load, operandFrom, _) |
     operandTo = operandFrom and ind = 0 and certain = true
     or
     numberOfLoadsFromOperand(load.getAUse(), operandTo, ind - 1, certain)
@@ -714,7 +716,7 @@ private predicate numberOfLoadsFromOperand(
 ) {
   numberOfLoadsFromOperandRec(operandFrom, operandTo, n, certain)
   or
-  not Ssa::isDereference(_, operandFrom) and
+  not Ssa::isDereference(_, operandFrom, _) and
   not conversionFlow(operandFrom, _, _, _) and
   operandFrom = operandTo and
   n = 0 and

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

@@ -74,7 +74,7 @@ predicate hasRawIndirectOperand(Operand op, int indirectionIndex) {
     type = getLanguageType(op) and
     m = countIndirectionsForCppType(type) and
     indirectionIndex = [1 .. m] and
-    not exists(getIRRepresentationOfIndirectOperand(op, indirectionIndex))
+    not hasIRRepresentationOfIndirectOperand(op, indirectionIndex, _, _)
   )
 }
 
@@ -88,7 +88,7 @@ predicate hasRawIndirectInstruction(Instruction instr, int indirectionIndex) {
     type = getResultLanguageType(instr) and
     m = countIndirectionsForCppType(type) and
     indirectionIndex = [1 .. m] and
-    not exists(getIRRepresentationOfIndirectInstruction(instr, indirectionIndex))
+    not hasIRRepresentationOfIndirectInstruction(instr, indirectionIndex, _, _)
   )
 }
 
@@ -610,7 +610,7 @@ private predicate indirectConversionFlowStep(Node nFrom, Node nTo) {
       hasOperandAndIndex(nFrom, op1, pragma[only_bind_into](indirectionIndex)) and
       hasOperandAndIndex(nTo, op2, indirectionIndex - 1) and
       instr = op2.getDef() and
-      isDereference(instr, op1)
+      isDereference(instr, op1, _)
     )
   )
 }
@@ -684,8 +684,41 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) {
   )
 }
 
-private predicate isArgumentOfCallable(DataFlowCall call, ArgumentNode arg) {
-  arg.argumentOf(call, _)
+private predicate isArgumentOfCallableInstruction(DataFlowCall call, Instruction instr) {
+  isArgumentOfCallableOperand(call, unique( | | getAUse(instr)))
+}
+
+private predicate isArgumentOfCallableOperand(DataFlowCall call, Operand operand) {
+  operand.(ArgumentOperand).getCall() = call
+  or
+  exists(FieldAddressInstruction fai |
+    fai.getObjectAddressOperand() = operand and
+    isArgumentOfCallableInstruction(call, fai)
+  )
+  or
+  exists(Instruction deref |
+    isArgumentOfCallableInstruction(call, deref) and
+    isDereference(deref, operand, _)
+  )
+  or
+  exists(Instruction instr |
+    isArgumentOfCallableInstruction(call, instr) and
+    conversionFlow(operand, instr, _, _)
+  )
+}
+
+private predicate isArgumentOfCallable(DataFlowCall call, Node n) {
+  isArgumentOfCallableOperand(call, n.asOperand())
+  or
+  exists(Operand op |
+    n.(IndirectOperand).hasOperandAndIndirectionIndex(op, _) and
+    isArgumentOfCallableOperand(call, op)
+  )
+  or
+  exists(Instruction instr |
+    n.(IndirectInstruction).hasInstructionAndIndirectionIndex(instr, _) and
+    isArgumentOfCallableInstruction(call, instr)
+  )
 }
 
 /** Holds if there is def-use or use-use flow from `pun` to `nodeTo`. */

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

@@ -320,10 +320,20 @@ private module IteratorIndirections {
   }
 }
 
-predicate isDereference(Instruction deref, Operand address) {
-  any(Indirection ind).isAdditionalDereference(deref, address)
+/**
+ * Holds if `deref` is the result of loading the value at the address
+ * represented by `address`.
+ *
+ * If `additional = true` then the dereference comes from an `Indirection`
+ * class (such as a call to an iterator's `operator*`), and if
+ * `additional = false` the dereference is a `LoadInstruction`.
+ */
+predicate isDereference(Instruction deref, Operand address, boolean additional) {
+  any(Indirection ind).isAdditionalDereference(deref, address) and
+  additional = true
   or
-  deref.(LoadInstruction).getSourceAddressOperand() = address
+  deref.(LoadInstruction).getSourceAddressOperand() = address and
+  additional = false
 }
 
 predicate isWrite(Node0Impl value, Operand address, boolean certain) {
@@ -545,7 +555,7 @@ private module Cached {
       isDef(_, value, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
       isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
       iteratorBase.getResultType() instanceof Interfaces::Iterator and
-      isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse()) and
+      isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse(), _) and
       memory = read.getSideEffectOperand().getAnyDef()
     )
   }
@@ -781,11 +791,14 @@ private module Cached {
    * instead associated with the operand returned by this predicate.
    */
   cached
-  Operand getIRRepresentationOfIndirectOperand(Operand operand, int indirectionIndex) {
+  predicate hasIRRepresentationOfIndirectOperand(
+    Operand operand, int indirectionIndex, Operand operandRepr, int indirectionIndexRepr
+  ) {
+    indirectionIndex = [1 .. countIndirectionsForCppType(getLanguageType(operand))] and
     exists(Instruction load |
-      isDereference(load, operand) and
-      result = unique( | | getAUse(load)) and
-      isUseImpl(operand, _, indirectionIndex - 1)
+      isDereference(load, operand, false) and
+      operandRepr = unique( | | getAUse(load)) and
+      indirectionIndexRepr = indirectionIndex - 1
     )
   }
 
@@ -797,12 +810,15 @@ private module Cached {
    * instead associated with the instruction returned by this predicate.
    */
   cached
-  Instruction getIRRepresentationOfIndirectInstruction(Instruction instr, int indirectionIndex) {
+  predicate hasIRRepresentationOfIndirectInstruction(
+    Instruction instr, int indirectionIndex, Instruction instrRepr, int indirectionIndexRepr
+  ) {
+    indirectionIndex = [1 .. countIndirectionsForCppType(getResultLanguageType(instr))] and
     exists(Instruction load, Operand address |
       address.getDef() = instr and
-      isDereference(load, address) and
-      isUseImpl(address, _, indirectionIndex - 1) and
-      result = load
+      isDereference(load, address, false) and
+      instrRepr = load and
+      indirectionIndexRepr = indirectionIndex - 1
     )
   }
 
@@ -823,7 +839,7 @@ private module Cached {
     or
     exists(int ind0 |
       exists(Operand address |
-        isDereference(operand.getDef(), address) and
+        isDereference(operand.getDef(), address, _) and
         isUseImpl(address, base, ind0)
       )
       or
@@ -893,7 +909,7 @@ private module Cached {
     )
     or
     exists(Operand address, boolean certain0 |
-      isDereference(operand.getDef(), address) and
+      isDereference(operand.getDef(), address, _) and
       isDefImpl(address, base, ind - 1, certain0)
     |
       if isCertainAddress(operand) then certain = certain0 else certain = false

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

@@ -57,7 +57,7 @@ private predicate operandToInstructionTaintStep(Operand opFrom, Instruction inst
   )
   or
   // Taint flow from an address to its dereference.
-  Ssa::isDereference(instrTo, opFrom)
+  Ssa::isDereference(instrTo, opFrom, _)
   or
   // Unary instructions tend to preserve enough information in practice that we
   // want taint to flow through.

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/array-access/ArrayAccessProductFlow.expected

@@ -9,44 +9,35 @@ edges
 | test.cpp:22:5:22:7 | arr indirection [p] | test.cpp:19:9:19:16 | mk_array indirection [p] |
 | test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:31:9:31:11 | arr indirection [p] |
 | test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:35:9:35:11 | arr indirection [p] |
-| test.cpp:31:9:31:11 | arr indirection [p] | test.cpp:31:13:31:13 | p indirection |
-| test.cpp:31:13:31:13 | p indirection | test.cpp:31:13:31:13 | p |
-| test.cpp:35:9:35:11 | arr indirection [p] | test.cpp:35:13:35:13 | p indirection |
-| test.cpp:35:13:35:13 | p indirection | test.cpp:35:13:35:13 | p |
+| test.cpp:31:9:31:11 | arr indirection [p] | test.cpp:31:13:31:13 | p |
+| test.cpp:35:9:35:11 | arr indirection [p] | test.cpp:35:13:35:13 | p |
 | test.cpp:39:27:39:29 | arr [p] | test.cpp:41:9:41:11 | arr indirection [p] |
 | test.cpp:39:27:39:29 | arr [p] | test.cpp:45:9:45:11 | arr indirection [p] |
-| test.cpp:41:9:41:11 | arr indirection [p] | test.cpp:41:13:41:13 | p indirection |
-| test.cpp:41:13:41:13 | p indirection | test.cpp:41:13:41:13 | p |
-| test.cpp:45:9:45:11 | arr indirection [p] | test.cpp:45:13:45:13 | p indirection |
-| test.cpp:45:13:45:13 | p indirection | test.cpp:45:13:45:13 | p |
+| test.cpp:41:9:41:11 | arr indirection [p] | test.cpp:41:13:41:13 | p |
+| test.cpp:45:9:45:11 | arr indirection [p] | test.cpp:45:13:45:13 | p |
 | test.cpp:50:18:50:25 | call to mk_array [p] | test.cpp:39:27:39:29 | arr [p] |
 | test.cpp:55:5:55:24 | ... = ... | test.cpp:55:9:55:9 | arr indirection [post update] [p] |
 | test.cpp:55:9:55:9 | arr indirection [post update] [p] | test.cpp:56:5:56:7 | arr indirection [p] |
 | test.cpp:55:13:55:18 | call to malloc | test.cpp:55:5:55:24 | ... = ... |
 | test.cpp:56:5:56:7 | arr indirection [p] | test.cpp:59:9:59:11 | arr indirection [p] |
 | test.cpp:56:5:56:7 | arr indirection [p] | test.cpp:63:9:63:11 | arr indirection [p] |
-| test.cpp:59:9:59:11 | arr indirection [p] | test.cpp:59:13:59:13 | p indirection |
-| test.cpp:59:13:59:13 | p indirection | test.cpp:59:13:59:13 | p |
-| test.cpp:63:9:63:11 | arr indirection [p] | test.cpp:63:13:63:13 | p indirection |
-| test.cpp:63:13:63:13 | p indirection | test.cpp:63:13:63:13 | p |
+| test.cpp:59:9:59:11 | arr indirection [p] | test.cpp:59:13:59:13 | p |
+| test.cpp:63:9:63:11 | arr indirection [p] | test.cpp:63:13:63:13 | p |
 | test.cpp:67:10:67:19 | mk_array_p indirection [p] | test.cpp:76:20:76:29 | call to mk_array_p indirection [p] |
 | test.cpp:67:10:67:19 | mk_array_p indirection [p] | test.cpp:98:18:98:27 | call to mk_array_p indirection [p] |
 | test.cpp:69:5:69:25 | ... = ... | test.cpp:69:10:69:10 | arr indirection [post update] [p] |
 | test.cpp:69:10:69:10 | arr indirection [post update] [p] | test.cpp:70:5:70:7 | arr indirection [p] |
 | test.cpp:69:14:69:19 | call to malloc | test.cpp:69:5:69:25 | ... = ... |
 | test.cpp:70:5:70:7 | arr indirection [p] | test.cpp:67:10:67:19 | mk_array_p indirection [p] |
+| test.cpp:70:5:70:7 | arr indirection [p] | test.cpp:70:5:70:7 | arr indirection [p] |
 | test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | test.cpp:79:9:79:11 | arr indirection [p] |
 | test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | test.cpp:83:9:83:11 | arr indirection [p] |
-| test.cpp:79:9:79:11 | arr indirection [p] | test.cpp:79:14:79:14 | p indirection |
-| test.cpp:79:14:79:14 | p indirection | test.cpp:79:14:79:14 | p |
-| test.cpp:83:9:83:11 | arr indirection [p] | test.cpp:83:14:83:14 | p indirection |
-| test.cpp:83:14:83:14 | p indirection | test.cpp:83:14:83:14 | p |
+| test.cpp:79:9:79:11 | arr indirection [p] | test.cpp:79:14:79:14 | p |
+| test.cpp:83:9:83:11 | arr indirection [p] | test.cpp:83:14:83:14 | p |
 | test.cpp:87:28:87:30 | arr indirection [p] | test.cpp:89:9:89:11 | arr indirection [p] |
 | test.cpp:87:28:87:30 | arr indirection [p] | test.cpp:93:9:93:11 | arr indirection [p] |
-| test.cpp:89:9:89:11 | arr indirection [p] | test.cpp:89:14:89:14 | p indirection |
-| test.cpp:89:14:89:14 | p indirection | test.cpp:89:14:89:14 | p |
-| test.cpp:93:9:93:11 | arr indirection [p] | test.cpp:93:14:93:14 | p indirection |
-| test.cpp:93:14:93:14 | p indirection | test.cpp:93:14:93:14 | p |
+| test.cpp:89:9:89:11 | arr indirection [p] | test.cpp:89:14:89:14 | p |
+| test.cpp:93:9:93:11 | arr indirection [p] | test.cpp:93:14:93:14 | p |
 | test.cpp:98:18:98:27 | call to mk_array_p indirection [p] | test.cpp:87:28:87:30 | arr indirection [p] |
 nodes
 | test.cpp:4:17:4:22 | call to malloc | semmle.label | call to malloc |
@@ -60,28 +51,22 @@ nodes
 | test.cpp:28:19:28:26 | call to mk_array [p] | semmle.label | call to mk_array [p] |
 | test.cpp:31:9:31:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:31:13:31:13 | p | semmle.label | p |
-| test.cpp:31:13:31:13 | p indirection | semmle.label | p indirection |
 | test.cpp:35:9:35:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:35:13:35:13 | p | semmle.label | p |
-| test.cpp:35:13:35:13 | p indirection | semmle.label | p indirection |
 | test.cpp:39:27:39:29 | arr [p] | semmle.label | arr [p] |
 | test.cpp:41:9:41:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:41:13:41:13 | p | semmle.label | p |
-| test.cpp:41:13:41:13 | p indirection | semmle.label | p indirection |
 | test.cpp:45:9:45:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:45:13:45:13 | p | semmle.label | p |
-| test.cpp:45:13:45:13 | p indirection | semmle.label | p indirection |
 | test.cpp:50:18:50:25 | call to mk_array [p] | semmle.label | call to mk_array [p] |
 | test.cpp:55:5:55:24 | ... = ... | semmle.label | ... = ... |
 | test.cpp:55:9:55:9 | arr indirection [post update] [p] | semmle.label | arr indirection [post update] [p] |
 | test.cpp:55:13:55:18 | call to malloc | semmle.label | call to malloc |
 | test.cpp:56:5:56:7 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:59:9:59:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:59:13:59:13 | p | semmle.label | p |
-| test.cpp:59:13:59:13 | p indirection | semmle.label | p indirection |
 | test.cpp:63:9:63:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:63:13:63:13 | p | semmle.label | p |
-| test.cpp:63:13:63:13 | p indirection | semmle.label | p indirection |
 | test.cpp:67:10:67:19 | mk_array_p indirection [p] | semmle.label | mk_array_p indirection [p] |
 | test.cpp:69:5:69:25 | ... = ... | semmle.label | ... = ... |
 | test.cpp:69:10:69:10 | arr indirection [post update] [p] | semmle.label | arr indirection [post update] [p] |
@@ -90,17 +75,13 @@ nodes
 | test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | semmle.label | call to mk_array_p indirection [p] |
 | test.cpp:79:9:79:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:79:14:79:14 | p | semmle.label | p |
-| test.cpp:79:14:79:14 | p indirection | semmle.label | p indirection |
 | test.cpp:83:9:83:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:83:14:83:14 | p | semmle.label | p |
-| test.cpp:83:14:83:14 | p indirection | semmle.label | p indirection |
 | test.cpp:87:28:87:30 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:89:9:89:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:89:14:89:14 | p | semmle.label | p |
-| test.cpp:89:14:89:14 | p indirection | semmle.label | p indirection |
 | test.cpp:93:9:93:11 | arr indirection [p] | semmle.label | arr indirection [p] |
 | test.cpp:93:14:93:14 | p | semmle.label | p |
-| test.cpp:93:14:93:14 | p indirection | semmle.label | p indirection |
 | test.cpp:98:18:98:27 | call to mk_array_p indirection [p] | semmle.label | call to mk_array_p indirection [p] |
 subpaths
 #select

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

@@ -0,0 +1,14 @@
+namespace {
+  struct Foo {
+    char string[10];
+  };
+
+  void acquire(char*);
+
+  Foo* test_self_argument_flow() {
+    Foo *info;
+    acquire(info->string); // clean
+
+    return info;
+  }
+}

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

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

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

@@ -0,0 +1,33 @@
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+import TestUtilities.InlineExpectationsTest
+
+module TestConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source.getLocation().getFile().getBaseName() = "self_argument_flow.cpp" and
+    source.asDefiningArgument() =
+      any(Call call | call.getTarget().hasName("acquire")).getAnArgument()
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink.asIndirectArgument() = any(Call call | call.getTarget().hasName("acquire")).getAnArgument()
+  }
+}
+
+import DataFlow::Global<TestConfig>
+
+module TestSelfArgumentFlow implements TestSig {
+  string getARelevantTag() { result = "self-arg-flow" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(DataFlow::Node sink |
+      flowTo(sink) and
+      location = sink.getLocation() and
+      element = sink.toString() and
+      tag = "self-arg-flow" and
+      value = ""
+    )
+  }
+}
+
+import MakeTest<TestSelfArgumentFlow>

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

@@ -14,6 +14,8 @@
 | ref.cpp:120:17:120:18 | x3 | ref.cpp:129:10:129:11 | x3 |
 | ref.cpp:120:21:120:22 | x4 | ref.cpp:131:15:131:16 | x4 |
 | ref.cpp:120:21:120:22 | x4 | ref.cpp:132:10:132:11 | x4 |
+| self_argument_flow.cpp:9:10:9:13 | info | self_argument_flow.cpp:10:13:10:16 | info |
+| self_argument_flow.cpp:9:10:9:13 | info | self_argument_flow.cpp:12:12:12:15 | info |
 | test.cpp:75:7:75:8 | u1 | test.cpp:76:8:76:9 | u1 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:84:13:84:14 | u2 |
 | test.cpp:83:7:83:8 | u2 | test.cpp:85:8:85:9 | u2 |