C++: Add PostUpdateNode for updates to structs with no chi node

MathiasVP · MathiasVP · commit ae1a30e817c1 · 2020-04-19T21:22:01.000+02:00
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -180,12 +180,16 @@ private class ArrayContent extends Content, TArrayContent {
   override Type getType() { none() }
 }
 
-/**
- * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
- * Thus, `node2` references an object with a field `f` that contains the
- * value of `node1`.
- */
-predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
+private predicate storeStepNoChi(Node node1, Content f, PostUpdateNode node2) {
+  exists(FieldAddressInstruction fa, StoreInstruction store |
+    store = node2.asInstruction() and
+    store.getDestinationAddress() = fa and
+    store.getSourceValue() = node1.asInstruction() and
+    f.(FieldContent).getField() = fa.getField()
+  )
+}
+
+private predicate storeStepChi(Node node1, Content f, PostUpdateNode node2) {
   exists(FieldAddressInstruction fa, StoreInstruction store |
     node1.asInstruction() = store and
     store.getDestinationAddress() = fa and
@@ -194,6 +198,16 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
   )
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
+ * Thus, `node2` references an object with a field `f` that contains the
+ * value of `node1`.
+ */
+predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
+  storeStepNoChi(node1, f, node2) or
+  storeStepChi(node1, f, node2)
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of `f`.
  * Thus, `node1` references an object with a field `f` whose value ends up in
diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
@@ -270,6 +270,19 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
 }
 
+private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
+  override StoreInstruction instr;
+
+  ExplicitSingleFieldStoreQualifierNode() {
+    exists(FieldAddressInstruction field |
+      field = instr.getDestinationAddress() and
+      not exists(ChiInstruction chi | chi.getPartial() = instr)
+    )
+  }
+
+  override Node getPreUpdateNode() { none() }
+}
+
 /**
  * A node that represents the value of a variable after a function call that
  * may have changed the variable because it's passed by reference.
@@ -404,6 +417,8 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
 }
 
+private predicate hasSize(Type t, int size) { t.getSize() = size }
+
 cached
 private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo) {
   iTo.(CopyInstruction).getSourceValue() = iFrom
@@ -452,6 +467,13 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
     iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
   )
   or
+  iTo.(CopyInstruction).getSourceValueOperand().getAnyDef() = iFrom and
+  exists(Class c, int size |
+    c = iTo.getResultType() and
+    hasSize(c, size) and
+    hasSize(iFrom.getResultType(), size)
+  )
+  or
   // Flow through modeled functions
   modelFlow(iFrom, iTo)
 }
