Merge pull request #10769 from hvitved/csharp/cil-ssa-data-flow-nodes

C#: Include CIL SSA definitions in `DataFlow::Node`

Author: hvitved

csharp/ql/lib/semmle/code/cil/DataFlow.qll

@@ -20,16 +20,18 @@ class DataFlowNode extends @cil_dataflow_node {
    * Holds if this node flows to `sink` in one step.
    * `tt` is the tainting that occurs during this step.
    */
-  predicate getALocalFlowSucc(DataFlowNode sink, TaintType tt) {
+  deprecated predicate getALocalFlowSucc(DataFlowNode sink, TaintType tt) {
     localExactStep(this, sink) and tt = TExactValue()
     or
     localTaintStep(this, sink) and tt = TTaintedValue()
   }
 
-  private predicate flowsToStep(DataFlowNode sink) { this.getALocalFlowSucc(sink, TExactValue()) }
+  deprecated private predicate flowsToStep(DataFlowNode sink) {
+    this.getALocalFlowSucc(sink, TExactValue())
+  }
 
   /** Holds if this node flows to `sink` in zero or more steps. */
-  predicate flowsTo(DataFlowNode sink) { this.flowsToStep*(sink) }
+  deprecated predicate flowsTo(DataFlowNode sink) { this.flowsToStep*(sink) }
 
   /** Gets the method that contains this dataflow node. */
   Method getMethod() { none() }
@@ -38,12 +40,12 @@ class DataFlowNode extends @cil_dataflow_node {
   Location getLocation() { none() }
 }
 
-private newtype TTaintType =
+deprecated private newtype TTaintType =
   TExactValue() or
   TTaintedValue()
 
 /** Describes how data is tainted. */
-class TaintType extends TTaintType {
+deprecated class TaintType extends TTaintType {
   string toString() {
     this = TExactValue() and result = "exact"
     or
@@ -52,12 +54,12 @@ class TaintType extends TTaintType {
 }
 
 /** A taint type where the data is untainted. */
-class Untainted extends TaintType, TExactValue { }
+deprecated class Untainted extends TaintType, TExactValue { }
 
 /** A taint type where the data is tainted. */
-class Tainted extends TaintType, TTaintedValue { }
+deprecated class Tainted extends TaintType, TTaintedValue { }
 
-private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
+deprecated private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
   exists(Ssa::Definition def, BasicBlock bb, int i | phi.hasLastInputRef(def, bb, i) |
     def.definesAt(_, bb, i) and
     input = def.getVariableUpdate().getSource()
@@ -76,7 +78,7 @@ private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
   )
 }
 
-private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
+deprecated private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(Opcodes::Dup).getAnOperand()
   or
   exists(Ssa::Definition def, VariableUpdate vu |
@@ -103,7 +105,7 @@ private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(ConditionalBranch).getAnOperand()
 }
 
-private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
+deprecated private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(BinaryArithmeticExpr).getAnOperand() or
   src = sink.(Opcodes::Neg).getOperand() or
   src = sink.(UnaryBitwiseOperation).getOperand()

csharp/ql/lib/semmle/code/cil/Method.qll

@@ -270,7 +270,7 @@ class Setter extends Accessor {
  */
 class TrivialSetter extends Method {
   TrivialSetter() {
-    exists(MethodImplementation impl | impl = this.getImplementation() |
+    exists(MethodImplementation impl | impl = this.getAnImplementation() |
       impl.getInstruction(0) instanceof ThisAccess and
       impl.getInstruction(1).(ParameterReadAccess).getTarget().getIndex() = 1 and
       impl.getInstruction(2) instanceof FieldWriteAccess

csharp/ql/lib/semmle/code/cil/Ssa.qll

@@ -24,10 +24,10 @@ module Ssa {
     }
 
     /** Gets a first read of this SSA definition. */
-    final ReadAccess getAFirstRead() { result = SsaImpl::getAFirstRead(this) }
+    deprecated final ReadAccess getAFirstRead() { result = SsaImpl::getAFirstRead(this) }
 
     /** Holds if `first` and `second` are adjacent reads of this SSA definition. */
-    final predicate hasAdjacentReads(ReadAccess first, ReadAccess second) {
+    deprecated final predicate hasAdjacentReads(ReadAccess first, ReadAccess second) {
       SsaImpl::hasAdjacentReads(this, first, second)
     }
 
@@ -58,8 +58,9 @@ module Ssa {
      * index `i` in basic block `bb` can reach this phi node without going through
      * other references.
      */
-    final predicate hasLastInputRef(Definition def, BasicBlock bb, int i) {
-      SsaImpl::hasLastInputRef(this, def, bb, i)
+    deprecated final predicate hasLastInputRef(Definition def, BasicBlock bb, int i) {
+      SsaImpl::lastRefRedef(def, bb, i, this) and
+      def = SsaImpl::getAPhiInput(this)
     }
   }
 }

csharp/ql/lib/semmle/code/cil/Stubs.qll

@@ -29,14 +29,17 @@ private module Cached {
 
   cached
   predicate bestImplementation(MethodImplementation mi) {
-    not assemblyIsStubImpl(mi.getLocation()) and
-    not exists(MethodImplementation better | mi.getMethod() = better.getMethod() |
-      mi.getNumberOfInstructions() < better.getNumberOfInstructions()
-      or
-      mi.getNumberOfInstructions() = better.getNumberOfInstructions() and
-      mi.getLocation().getFile().toString() > better.getLocation().getFile().toString()
-    ) and
-    exists(mi.getAnInstruction())
+    exists(Assembly asm |
+      asm = mi.getLocation() and
+      (assemblyIsStubImpl(asm) implies asm.getFile().extractedQlTest()) and
+      not exists(MethodImplementation better | mi.getMethod() = better.getMethod() |
+        mi.getNumberOfInstructions() < better.getNumberOfInstructions()
+        or
+        mi.getNumberOfInstructions() = better.getNumberOfInstructions() and
+        asm.getFile().toString() > better.getLocation().getFile().toString()
+      ) and
+      exists(mi.getAnInstruction())
+    )
   }
 }
 

csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll

@@ -68,9 +68,8 @@ private module Cached {
   Definition getAPhiInput(PhiNode phi) { phiHasInputFromBlock(phi, result, _) }
 
   cached
-  predicate hasLastInputRef(Definition phi, Definition def, BasicBlock bb, int i) {
-    lastRefRedef(def, bb, i, phi) and
-    def = getAPhiInput(phi)
+  predicate lastRefBeforeRedef(Definition def, BasicBlock bb, int i, Definition next) {
+    lastRefRedef(def, bb, i, next)
   }
 }
 

csharp/ql/lib/semmle/code/csharp/commons/Disposal.qll

@@ -11,15 +11,19 @@ private predicate isDisposeMethod(DotNet::Callable method) {
   method.getNumberOfParameters() = 0
 }
 
-private predicate cilVariableReadFlowsTo(CIL::Variable variable, CIL::DataFlowNode n) {
-  n = variable.getARead()
+private predicate cilVariableReadFlowsToNode(CIL::Variable variable, DataFlow::Node n) {
+  n.asExpr() = variable.getARead()
   or
-  exists(CIL::DataFlowNode mid |
-    cilVariableReadFlowsTo(variable, mid) and
-    mid.getALocalFlowSucc(n, any(CIL::Untainted u))
+  exists(DataFlow::Node mid |
+    cilVariableReadFlowsToNode(variable, mid) and
+    DataFlow::localFlowStep(mid, n)
   )
 }
 
+private predicate cilVariableReadFlowsTo(CIL::Variable variable, CIL::DataFlowNode n) {
+  cilVariableReadFlowsToNode(variable, DataFlow::exprNode(n))
+}
+
 private predicate disposedCilVariable(CIL::Variable variable) {
   // `variable` is the `this` parameter on a dispose method.
   isDisposeMethod(variable.(CIL::ThisParameter).getMethod())

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -17,6 +17,7 @@ private import semmle.code.csharp.frameworks.EntityFramework
 private import semmle.code.csharp.frameworks.NHibernate
 private import semmle.code.csharp.frameworks.system.Collections
 private import semmle.code.csharp.frameworks.system.threading.Tasks
+private import semmle.code.cil.Ssa::Ssa as CilSsa
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(NodeImpl n) { result = n.getEnclosingCallableImpl() }
@@ -177,6 +178,12 @@ predicate hasNodePath(ControlFlowReachabilityConfiguration conf, ExprNode n1, No
   )
 }
 
+/** Gets the CIL data-flow node for `node`, if any. */
+CIL::DataFlowNode asCilDataFlowNode(Node node) {
+  result = node.asParameter() or
+  result = node.asExpr()
+}
+
 /** Provides predicates related to local data flow. */
 module LocalFlow {
   private class LocalExprStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -281,15 +288,6 @@ module LocalFlow {
     }
   }
 
-  private CIL::DataFlowNode asCilDataFlowNode(Node node) {
-    result = node.asParameter() or
-    result = node.asExpr()
-  }
-
-  private predicate localFlowStepCil(Node nodeFrom, Node nodeTo) {
-    asCilDataFlowNode(nodeFrom).getALocalFlowSucc(asCilDataFlowNode(nodeTo), any(CIL::Untainted t))
-  }
-
   /**
    * An uncertain SSA definition. Either an uncertain explicit definition or an
    * uncertain qualifier definition.
@@ -341,7 +339,7 @@ module LocalFlow {
 
   /**
    * Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
-   * SSA definition `def.
+   * SSA definition `def`.
    */
   predicate localSsaFlowStep(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
     // Flow from SSA definition/parameter to first read
@@ -386,6 +384,76 @@ module LocalFlow {
     )
   }
 
+  private module CilFlow {
+    private import semmle.code.cil.internal.SsaImpl as CilSsaImpl
+
+    /**
+     * Holds if `nodeFrom` is a last node referencing SSA definition `def`, which
+     * can reach `next`.
+     */
+    private predicate localFlowCilSsaInput(
+      Node nodeFrom, CilSsa::Definition def, CilSsa::Definition next
+    ) {
+      exists(CIL::BasicBlock bb, int i | CilSsaImpl::lastRefBeforeRedef(def, bb, i, next) |
+        def.definesAt(_, bb, i) and
+        def = nodeFrom.(CilSsaDefinitionNode).getDefinition()
+        or
+        nodeFrom = TCilExprNode(bb.getNode(i).(CIL::ReadAccess))
+      )
+    }
+
+    /**
+     * Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
+     * CIL SSA definition `def`.
+     */
+    private predicate localCilSsaFlowStep(CilSsa::Definition def, Node nodeFrom, Node nodeTo) {
+      // Flow into SSA definition
+      exists(CIL::VariableUpdate vu |
+        vu = def.getVariableUpdate() and
+        vu.getSource() = asCilDataFlowNode(nodeFrom) and
+        def = nodeTo.(CilSsaDefinitionNode).getDefinition()
+      )
+      or
+      // Flow from SSA definition to first read
+      def = nodeFrom.(CilSsaDefinitionNode).getDefinition() and
+      nodeTo = TCilExprNode(CilSsaImpl::getAFirstRead(def))
+      or
+      // Flow from read to next read
+      exists(CIL::ReadAccess readFrom, CIL::ReadAccess readTo |
+        CilSsaImpl::hasAdjacentReads(def, readFrom, readTo) and
+        nodeTo = TCilExprNode(readTo) and
+        nodeFrom = TCilExprNode(readFrom)
+      )
+      or
+      // Flow into phi node
+      exists(CilSsa::PhiNode phi |
+        localFlowCilSsaInput(nodeFrom, def, phi) and
+        phi = nodeTo.(CilSsaDefinitionNode).getDefinition() and
+        def = CilSsaImpl::getAPhiInput(phi)
+      )
+    }
+
+    private predicate localExactStep(CIL::DataFlowNode src, CIL::DataFlowNode sink) {
+      src = sink.(CIL::Opcodes::Dup).getAnOperand()
+      or
+      src = sink.(CIL::Conversion).getExpr()
+      or
+      src = sink.(CIL::WriteAccess).getExpr()
+      or
+      src = sink.(CIL::Method).getAnImplementation().getAnInstruction().(CIL::Return)
+      or
+      src = sink.(CIL::Return).getExpr()
+      or
+      src = sink.(CIL::ConditionalBranch).getAnOperand()
+    }
+
+    predicate localFlowStepCil(Node nodeFrom, Node nodeTo) {
+      localExactStep(asCilDataFlowNode(nodeFrom), asCilDataFlowNode(nodeTo))
+      or
+      localCilSsaFlowStep(_, nodeFrom, nodeTo)
+    }
+  }
+
   predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
     exists(Ssa::Definition def |
       localSsaFlowStep(def, nodeFrom, nodeTo) and
@@ -398,7 +466,7 @@ module LocalFlow {
     or
     ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
     or
-    localFlowStepCil(nodeFrom, nodeTo)
+    CilFlow::localFlowStepCil(nodeFrom, nodeTo)
   }
 
   /**
@@ -719,6 +787,7 @@ private module Cached {
       cfn.getElement() instanceof Expr
     } or
     TCilExprNode(CIL::Expr e) { e.getImplementation() instanceof CIL::BestImplementation } or
+    TCilSsaDefinitionNode(CilSsa::Definition def) or
     TSsaDefinitionNode(Ssa::Definition def) {
       // Handled by `TExplicitParameterNode` below
       not def.(Ssa::ExplicitDefinition).getADefinition() instanceof
@@ -867,6 +936,28 @@ predicate nodeIsHidden(Node n) {
   n.asExpr() = any(WithExpr we).getInitializer()
 }
 
+/** A CIL SSA definition, viewed as a node in a data flow graph. */
+class CilSsaDefinitionNode extends NodeImpl, TCilSsaDefinitionNode {
+  CilSsa::Definition def;
+
+  CilSsaDefinitionNode() { this = TCilSsaDefinitionNode(def) }
+
+  /** Gets the underlying SSA definition. */
+  CilSsa::Definition getDefinition() { result = def }
+
+  override DataFlowCallable getEnclosingCallableImpl() {
+    result.asCallable() = def.getBasicBlock().getFirstNode().getImplementation().getMethod()
+  }
+
+  override CIL::Type getTypeImpl() { result = def.getSourceVariable().getType() }
+
+  override ControlFlow::Node getControlFlowNodeImpl() { none() }
+
+  override Location getLocationImpl() { result = def.getBasicBlock().getLocation() }
+
+  override string toStringImpl() { result = def.toString() }
+}
+
 /** An SSA definition, viewed as a node in a data flow graph. */
 class SsaDefinitionNode extends NodeImpl, TSsaDefinitionNode {
   Ssa::Definition def;

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll

@@ -161,7 +161,7 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
  * local (intra-procedural) steps.
  */
 pragma[inline]
-predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
+predicate localExprFlow(DotNet::Expr e1, DotNet::Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
 
 /**
  * A data flow node that jumps between callables. This can be extended in

csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

@@ -26,13 +26,14 @@ predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
 bindingset[node]
 predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::Content c) { none() }
 
-private CIL::DataFlowNode asCilDataFlowNode(DataFlow::Node node) {
-  result = node.asParameter() or
-  result = node.asExpr()
+private predicate localCilTaintStep(CIL::DataFlowNode src, CIL::DataFlowNode sink) {
+  src = sink.(CIL::BinaryArithmeticExpr).getAnOperand() or
+  src = sink.(CIL::Opcodes::Neg).getOperand() or
+  src = sink.(CIL::UnaryBitwiseOperation).getOperand()
 }
 
 private predicate localTaintStepCil(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-  asCilDataFlowNode(nodeFrom).getALocalFlowSucc(asCilDataFlowNode(nodeTo), any(CIL::Tainted t))
+  localCilTaintStep(asCilDataFlowNode(nodeFrom), asCilDataFlowNode(nodeTo))
 }
 
 private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {