Merge pull request github#3097 from jbj/detect-conflated-memory

C++: Implement Instruction.isResultConflated

Author: Dave Bartolomeo

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

@@ -183,7 +183,7 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   i2.(UnaryInstruction).getUnary() = i1
   or
   i2.(ChiInstruction).getPartial() = i1 and
-  not isChiForAllAliasedMemory(i2)
+  not i2.isResultConflated()
   or
   exists(BinaryInstruction bin |
     bin = i2 and
@@ -276,19 +276,6 @@ private predicate modelTaintToParameter(Function f, int parameterIn, int paramet
   )
 }
 
-/**
- * Holds if `chi` is on the chain of chi-instructions for all aliased memory.
- * Taint should not pass through these instructions since they tend to mix up
- * unrelated objects.
- */
-private predicate isChiForAllAliasedMemory(Instruction instr) {
-  instr.(ChiInstruction).getTotal() instanceof AliasedDefinitionInstruction
-  or
-  isChiForAllAliasedMemory(instr.(ChiInstruction).getTotal())
-  or
-  isChiForAllAliasedMemory(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
-}
-
 private predicate modelTaintToReturnValue(Function f, int parameterIn) {
   // Taint flow from parameter to return value
   exists(FunctionInput modelIn, FunctionOutput modelOut |

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRSanity.qll

@@ -274,6 +274,36 @@ module InstructionSanity {
     funcText = Language::getIdentityString(func.getFunction())
   }
 
+  /**
+   * Holds if `instr` is on the chain of chi/phi instructions for all aliased
+   * memory.
+   */
+  private predicate isOnAliasedDefinitionChain(Instruction instr) {
+    instr instanceof AliasedDefinitionInstruction
+    or
+    isOnAliasedDefinitionChain(instr.(ChiInstruction).getTotal())
+    or
+    isOnAliasedDefinitionChain(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
+  }
+
+  private predicate shouldBeConflated(Instruction instr) {
+    isOnAliasedDefinitionChain(instr)
+    or
+    instr instanceof UnmodeledDefinitionInstruction
+    or
+    instr.getOpcode() instanceof Opcode::InitializeNonLocal
+  }
+
+  query predicate notMarkedAsConflated(Instruction instr) {
+    shouldBeConflated(instr) and
+    not instr.isResultConflated()
+  }
+
+  query predicate wronglyMarkedAsConflated(Instruction instr) {
+    instr.isResultConflated() and
+    not shouldBeConflated(instr)
+  }
+
   query predicate invalidOverlap(
     MemoryOperand useOperand, string message, IRFunction func, string funcText
   ) {

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -338,6 +338,17 @@ class Instruction extends Construction::TInstruction {
     Construction::hasModeledMemoryResult(this)
   }
 
+  /**
+   * Holds if this is an instruction with a memory result that represents a
+   * conflation of more than one memory allocation.
+   *
+   * This happens in practice when dereferencing a pointer that cannot be
+   * tracked back to a single local allocation. Such memory is instead modeled
+   * as originating on the `AliasedDefinitionInstruction` at the entry of the
+   * function.
+   */
+  final predicate isResultConflated() { Construction::hasConflatedMemoryResult(this) }
+
   /**
    * Gets the successor of this instruction along the control flow edge
    * specified by `kind`.

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -354,6 +354,7 @@ class AllAliasedMemory extends TAllAliasedMemory, MemoryLocation {
   final override predicate isMayAccess() { isMayAccess = true }
 }
 
+/** A virtual variable that groups all escaped memory within a function. */
 class AliasedVirtualVariable extends AllAliasedMemory, VirtualVariable {
   AliasedVirtualVariable() { not isMayAccess() }
 }
@@ -429,10 +430,18 @@ private Overlap getExtentOverlap(MemoryLocation def, MemoryLocation use) {
       use instanceof EntireAllocationMemoryLocation and
       result instanceof MustExactlyOverlap
       or
-      // EntireAllocationMemoryLocation totally overlaps any ___location within the same virtual
-      // variable.
       not use instanceof EntireAllocationMemoryLocation and
-      result instanceof MustTotallyOverlap
+      if def.getAllocation() = use.getAllocation()
+      then
+        // EntireAllocationMemoryLocation totally overlaps any ___location within
+        // the same allocation.
+        result instanceof MustTotallyOverlap
+      else (
+        // There is no overlap with a ___location that's known to belong to a
+        // different allocation, but all other locations may partially overlap.
+        not exists(use.getAllocation()) and
+        result instanceof MayPartiallyOverlap
+      )
     )
     or
     exists(VariableMemoryLocation defVariableLocation |

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -65,6 +65,29 @@ private module Cached {
     instruction instanceof ChiInstruction // Chis always have modeled results
   }
 
+  cached
+  predicate hasConflatedMemoryResult(Instruction instruction) {
+    instruction instanceof UnmodeledDefinitionInstruction
+    or
+    instruction instanceof AliasedDefinitionInstruction
+    or
+    instruction.getOpcode() instanceof Opcode::InitializeNonLocal
+    or
+    // Chi instructions track virtual variables, and therefore a chi instruction is
+    // conflated if it's associated with the aliased virtual variable.
+    exists(OldInstruction oldInstruction | instruction = Chi(oldInstruction) |
+      Alias::getResultMemoryLocation(oldInstruction).getVirtualVariable() instanceof
+        Alias::AliasedVirtualVariable
+    )
+    or
+    // Phi instructions track locations, and therefore a phi instruction is
+    // conflated if it's associated with a conflated ___location.
+    exists(Alias::MemoryLocation ___location |
+      instruction = Phi(_, ___location) and
+      not exists(___location.getAllocation())
+    )
+  }
+
   cached
   Instruction getRegisterOperandDefinition(Instruction instruction, RegisterOperandTag tag) {
     exists(OldInstruction oldInstruction, OldIR::RegisterOperand oldOperand |

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRSanity.qll

@@ -274,6 +274,36 @@ module InstructionSanity {
     funcText = Language::getIdentityString(func.getFunction())
   }
 
+  /**
+   * Holds if `instr` is on the chain of chi/phi instructions for all aliased
+   * memory.
+   */
+  private predicate isOnAliasedDefinitionChain(Instruction instr) {
+    instr instanceof AliasedDefinitionInstruction
+    or
+    isOnAliasedDefinitionChain(instr.(ChiInstruction).getTotal())
+    or
+    isOnAliasedDefinitionChain(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
+  }
+
+  private predicate shouldBeConflated(Instruction instr) {
+    isOnAliasedDefinitionChain(instr)
+    or
+    instr instanceof UnmodeledDefinitionInstruction
+    or
+    instr.getOpcode() instanceof Opcode::InitializeNonLocal
+  }
+
+  query predicate notMarkedAsConflated(Instruction instr) {
+    shouldBeConflated(instr) and
+    not instr.isResultConflated()
+  }
+
+  query predicate wronglyMarkedAsConflated(Instruction instr) {
+    instr.isResultConflated() and
+    not shouldBeConflated(instr)
+  }
+
   query predicate invalidOverlap(
     MemoryOperand useOperand, string message, IRFunction func, string funcText
   ) {

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -338,6 +338,17 @@ class Instruction extends Construction::TInstruction {
     Construction::hasModeledMemoryResult(this)
   }
 
+  /**
+   * Holds if this is an instruction with a memory result that represents a
+   * conflation of more than one memory allocation.
+   *
+   * This happens in practice when dereferencing a pointer that cannot be
+   * tracked back to a single local allocation. Such memory is instead modeled
+   * as originating on the `AliasedDefinitionInstruction` at the entry of the
+   * function.
+   */
+  final predicate isResultConflated() { Construction::hasConflatedMemoryResult(this) }
+
   /**
    * Gets the successor of this instruction along the control flow edge
    * specified by `kind`.

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -61,6 +61,15 @@ private module Cached {
   cached
   predicate hasModeledMemoryResult(Instruction instruction) { none() }
 
+  cached
+  predicate hasConflatedMemoryResult(Instruction instruction) {
+    instruction instanceof UnmodeledDefinitionInstruction
+    or
+    instruction instanceof AliasedDefinitionInstruction
+    or
+    instruction.getOpcode() instanceof Opcode::InitializeNonLocal
+  }
+
   cached
   Expr getInstructionConvertedResultExpression(Instruction instruction) {
     exists(TranslatedExpr translatedExpr |

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRSanity.qll

@@ -274,6 +274,36 @@ module InstructionSanity {
     funcText = Language::getIdentityString(func.getFunction())
   }
 
+  /**
+   * Holds if `instr` is on the chain of chi/phi instructions for all aliased
+   * memory.
+   */
+  private predicate isOnAliasedDefinitionChain(Instruction instr) {
+    instr instanceof AliasedDefinitionInstruction
+    or
+    isOnAliasedDefinitionChain(instr.(ChiInstruction).getTotal())
+    or
+    isOnAliasedDefinitionChain(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
+  }
+
+  private predicate shouldBeConflated(Instruction instr) {
+    isOnAliasedDefinitionChain(instr)
+    or
+    instr instanceof UnmodeledDefinitionInstruction
+    or
+    instr.getOpcode() instanceof Opcode::InitializeNonLocal
+  }
+
+  query predicate notMarkedAsConflated(Instruction instr) {
+    shouldBeConflated(instr) and
+    not instr.isResultConflated()
+  }
+
+  query predicate wronglyMarkedAsConflated(Instruction instr) {
+    instr.isResultConflated() and
+    not shouldBeConflated(instr)
+  }
+
   query predicate invalidOverlap(
     MemoryOperand useOperand, string message, IRFunction func, string funcText
   ) {

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -338,6 +338,17 @@ class Instruction extends Construction::TInstruction {
     Construction::hasModeledMemoryResult(this)
   }
 
+  /**
+   * Holds if this is an instruction with a memory result that represents a
+   * conflation of more than one memory allocation.
+   *
+   * This happens in practice when dereferencing a pointer that cannot be
+   * tracked back to a single local allocation. Such memory is instead modeled
+   * as originating on the `AliasedDefinitionInstruction` at the entry of the
+   * function.
+   */
+  final predicate isResultConflated() { Construction::hasConflatedMemoryResult(this) }
+
   /**
    * Gets the successor of this instruction along the control flow edge
    * specified by `kind`.