C#: Restrict data flow for multi-bodied methods

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

@@ -239,7 +239,7 @@ abstract class DataFlowCall extends TDataFlowCall {
    * For more information, see
    * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
    */
-  final predicate hasLocationInfo(
+  deprecated final predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
     this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
@@ -248,10 +248,10 @@ abstract class DataFlowCall extends TDataFlowCall {
   /** Gets a best-effort total ordering. */
   int totalorder() {
     this =
-      rank[result](DataFlowCall c, int startline, int startcolumn |
-        c.hasLocationInfo(_, startline, startcolumn, _, _)
+      rank[result](DataFlowCall c, string filePath, int startline, int startcolumn |
+        c.getLocation().hasLocationInfo(filePath, startline, startcolumn, _, _)
       |
-        c order by startline, startcolumn
+        c order by filePath, startline, startcolumn
       )
   }
 }

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

@@ -29,4 +29,6 @@ module CsharpDataFlow implements InputSig<Location> {
   predicate neverSkipInPathGraph(Node n) {
     exists(n.(AssignableDefinitionNode).getDefinition().getTargetAccess())
   }
+
+  predicate getSecondLevelScope = Private::getSecondLevelScope/1;
 }

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

@@ -1235,6 +1235,18 @@ private module Cached {
   newtype TDataFlowType =
     TGvnDataFlowType(Gvn::GvnType t) or
     TDelegateDataFlowType(Callable lambda) { lambdaCreationExpr(_, lambda) }
+
+  cached
+  newtype TNodeRegion =
+    TBasicBlockRegion(ControlFlow::BasicBlock bb) or
+    TSecondLevelScopeRegion(DataFlowSecondLevelScope scope)
+
+  cached
+  predicate nodeRegionContains(NodeRegion nr, Node n) {
+    nr.asBasicBlock() = n.getControlFlowNode().getBasicBlock()
+    or
+    nr.asSecondLevelScope() = getSecondLevelScope(n)
+  }
 }
 
 import Cached
@@ -2380,17 +2392,53 @@ predicate expectsContent(Node n, ContentSet c) {
   n.asExpr() instanceof SpreadElementExpr and c instanceof ElementContent
 }
 
-class NodeRegion instanceof ControlFlow::BasicBlock {
+private class MultiBodyCallable extends Callable {
+  MultiBodyCallable() { strictcount(this.getBody()) > 1 }
+}
+
+bindingset[e1, e2]
+pragma[inline_late]
+private predicate inSameFile(Element e1, Element e2) { e1.getFile() = e2.getFile() }
+
+pragma[nomagic]
+private predicate callTargetsMultiBodyInSameFile(DataFlowCall call, MultiBodyCallable target) {
+  target = call.getARuntimeTarget().asCallable() and
+  inSameFile(call.getExpr(), target.getBody())
+}
+
+/**
+ * A method body belonging to a method with multiple bodies.
+ *
+ * Methods with multiple bodies are treated as virtual dispatch.
+ */
+class DataFlowSecondLevelScope extends ControlFlowElement {
+  DataFlowSecondLevelScope() { this = any(MultiBodyCallable c).getBody() }
+}
+
+DataFlowSecondLevelScope getSecondLevelScope(Node n) {
+  result.getAControlFlowNode().getBasicBlock().getASuccessor*() =
+    n.getControlFlowNode().getBasicBlock()
+}
+
+class NodeRegion extends TNodeRegion {
   string toString() { result = "NodeRegion" }
 
-  predicate contains(Node n) { this = n.getControlFlowNode().getBasicBlock() }
+  ControlFlow::BasicBlock asBasicBlock() { this = TBasicBlockRegion(result) }
+
+  DataFlowSecondLevelScope asSecondLevelScope() { this = TSecondLevelScopeRegion(result) }
+
+  predicate contains(Node n) { nodeRegionContains(this, n) }
 
   int totalOrder() {
     this =
-      rank[result](ControlFlow::BasicBlock b, int startline, int startcolumn |
-        b.getLocation().hasLocationInfo(_, startline, startcolumn, _, _)
+      rank[result](NodeRegion r, int i, string filePath, int startline, int startcolumn |
+        i = 0 and
+        r.asBasicBlock().getLocation().hasLocationInfo(filePath, startline, startcolumn, _, _)
+        or
+        i = 1 and
+        r.asSecondLevelScope().getLocation().hasLocationInfo(filePath, startline, startcolumn, _, _)
       |
-        b order by startline, startcolumn
+        r order by i, filePath, startline, startcolumn
       )
   }
 }
@@ -2404,7 +2452,16 @@ predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) {
   |
     viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
     paramNode.getSsaDefinition().getARead() = guard and
-    guard.controlsBlock(nr, bs, _)
+    guard.controlsBlock(nr.asBasicBlock(), bs, _)
+  )
+  or
+  // When a call targets a method with multiple bodies, and one of the bodies
+  // is in the same file as the call, we prohibit flow to all other bodies.
+  exists(MultiBodyCallable target, DataFlowSecondLevelScope scope |
+    callTargetsMultiBodyInSameFile(call, target) and
+    nr.asSecondLevelScope() = scope and
+    scope = target.getBody() and
+    not inSameFile(call.getExpr(), scope)
   )
 }
 
@@ -2896,8 +2953,6 @@ predicate knownSourceModel(Node source, string model) { sourceNode(source, _, mo
 
 predicate knownSinkModel(Node sink, string model) { sinkNode(sink, _, model) }
 
-class DataFlowSecondLevelScope = Unit;
-
 /**
  * Holds if flow is allowed to pass from parameter `p` and back to itself as a
  * side-effect, resulting in a summary from `p` to itself.