C++: Remove field conflation

Author: MathiasVP

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -1,6 +1,7 @@
 import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import semmle.code.cpp.ir.dataflow.DataFlow2
 private import semmle.code.cpp.ir.dataflow.DataFlow3
 private import semmle.code.cpp.ir.IR
@@ -228,6 +229,7 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Flow from an element to an array or union that contains it.
   i2.(ChiInstruction).getPartial() = i1 and
   not i2.isResultConflated() and
+  not exists(PartialDefinitionNode n | n.asInstruction() = i2) and
   exists(Type t | i2.getResultLanguageType().hasType(t, false) |
     t instanceof Union
     or

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

@@ -239,6 +239,8 @@ abstract class PostUpdateNode extends InstructionNode {
 }
 
 /**
+ * INTERNAL: do not use.
+ *
  * The base class for nodes that perform "partial definitions".
  *
  * In contrast to a normal "definition", which provides a new value for
@@ -251,7 +253,7 @@ abstract class PostUpdateNode extends InstructionNode {
  * setY(&x); // a partial definition of the object `x`.
  * ```
  */
-abstract private class PartialDefinitionNode extends PostUpdateNode, TInstructionNode { }
+abstract class PartialDefinitionNode extends PostUpdateNode, TInstructionNode { }
 
 private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override ChiInstruction instr;
@@ -270,6 +272,17 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
 }
 
+private class FieldStoreWriteSideEffectNode extends PartialDefinitionNode {
+  override ChiInstruction instr;
+
+  FieldStoreWriteSideEffectNode() {
+    not instr.isResultConflated() and
+    exists(WriteSideEffectInstruction sideEffect | instr.getPartial() = sideEffect)
+  }
+
+  override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
+}
+
 /**
  * Not every store instruction generates a chi instruction that we can attach a PostUpdateNode to.
  * For instance, an update to a field of a struct containing only one field. For these cases we
@@ -421,6 +434,32 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFr
  */
 predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
   simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
+  or
+  // The next two rules allow flow from partial definitions in setters to succeeding loads in the caller.
+  // First, we add flow from write side-effects to non-conflated chi instructions through their
+  // partial operands. Consider the following example:
+  // ```
+  // void setX(Point* p, int new_x) {
+  //   p->x = new_x;
+  // }
+  // ...
+  // setX(&p, taint());
+  // ```
+  // Here, a `WriteSideEffectInstruction` will provide a new definition for `p->x` after the call to
+  // `setX`, which will be melded into `p` through a chi instruction.
+  nodeTo instanceof FieldStoreWriteSideEffectNode and
+  exists(ChiInstruction chi | chi = nodeTo.asInstruction() |
+    chi.getPartialOperand().getDef() = nodeFrom.asInstruction().(WriteSideEffectInstruction) and
+    not chi.isResultConflated()
+  )
+  or
+  // Next, we add flow from non-conflated chi instructions to loads (even when they are not precise).
+  // This ensures that loads of `p->x` gets data flow from the `WriteSideEffectInstruction` above.
+  nodeFrom instanceof FieldStoreWriteSideEffectNode and
+  exists(ChiInstruction chi | chi = nodeFrom.asInstruction() |
+    not chi.isResultConflated() and
+    nodeTo.asInstruction().(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
+  )
 }
 
 pragma[noinline]
@@ -458,28 +497,6 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
   // for now.
   iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
   or
-  // The next two rules allow flow from partial definitions in setters to succeeding loads in the caller.
-  // First, we add flow from write side-effects to non-conflated chi instructions through their
-  // partial operands. Consider the following example:
-  // ```
-  // void setX(Point* p, int new_x) {
-  //   p->x = new_x;
-  // }
-  // ...
-  // setX(&p, taint());
-  // ```
-  // Here, a `WriteSideEffectInstruction` will provide a new definition for `p->x` after the call to
-  // `setX`, which will be melded into `p` through a chi instruction.
-  iTo.getAnOperand().(ChiPartialOperand).getDef() = iFrom.(WriteSideEffectInstruction) and
-  not iTo.isResultConflated()
-  or
-  // Next, we add flow from non-conflated chi instructions to loads (even when they are not precise).
-  // This ensures that loads of `p->x` gets data flow from the `WriteSideEffectInstruction` above.
-  exists(ChiInstruction chi | iFrom = chi |
-    not chi.isResultConflated() and
-    iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = chi
-  )
-  or
   // Flow from stores to structs with a single field to a load of that field.
   iTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = iFrom and
   exists(int size, Type type |