C#: Adapt to changes in FlowSummaryImpl

Author: hvitved

csharp/ql/lib/semmle/code/csharp/dataflow/FlowSummary.qll

@@ -18,6 +18,11 @@ deprecated module SummaryComponentStack = Impl::Private::SummaryComponentStack;
 
 deprecated class RequiredSummaryComponentStack = Impl::Private::RequiredSummaryComponentStack;
 
-class SummarizedCallable = Impl::Public::SummarizedCallable;
+/** Provides the `Range` class used to define the extent of `SummarizedCallable`. */
+module SummarizedCallable {
+  class Range = Impl::Public::SummarizedCallable;
+}
+
+final class SummarizedCallable = Impl::Public::RelevantSummarizedCallable;
 
 class Provenance = Impl::Public::Provenance;

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

@@ -371,24 +371,9 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
   /** Gets the underlying call. */
   DispatchCall getDispatchCall() { result = dc }
 
-  pragma[nomagic]
-  private predicate hasSourceTarget() { dc.getAStaticTarget().fromSource() }
-
   pragma[nomagic]
   private FlowSummary::SummarizedCallable getASummarizedCallableTarget() {
-    // Only use summarized callables with generated summaries in case
-    // we are not able to dispatch to a source declaration.
-    exists(boolean static |
-      result = this.getATarget(static) and
-      not (
-        result.applyGeneratedModel() and
-        this.hasSourceTarget()
-      )
-    |
-      static = false
-      or
-      static = true and not result instanceof RuntimeCallable
-    )
+    result = this.getATarget(_)
   }
 
   pragma[nomagic]

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

@@ -848,7 +848,7 @@ private predicate fieldOrPropertyStore(Expr e, ContentSet c, Expr src, Expr q, b
         FlowSummaryImpl::Private::SummarizedCallableImpl sc,
         FlowSummaryImpl::Private::SummaryComponentStack input, ContentSet readSet
       |
-        sc.propagatesFlow(input, _, _, _) and
+        sc.propagatesFlow(input, _, _, _, _, _) and
         input.contains(FlowSummaryImpl::Private::SummaryComponent::content(readSet)) and
         c.getAStoreContent() = readSet.getAReadContent()
       )
@@ -1021,7 +1021,6 @@ private class InstanceCallable extends Callable {
   private Location l;
 
   InstanceCallable() {
-    this = any(DataFlowCallable dfc).asCallable(l) and
     not this.(Modifiable).isStatic() and
     // local functions and delegate capture `this` and should therefore
     // not have a `this` parameter

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

@@ -349,20 +349,23 @@ private Declaration interpretExt(Declaration d, ExtPath ext) {
 /** Gets the source/sink/summary/neutral element corresponding to the supplied parameters. */
 pragma[nomagic]
 Declaration interpretElement(
-  string namespace, string type, boolean subtypes, string name, string signature, string ext
+  string namespace, string type, boolean subtypes, string name, string signature, string ext,
+  boolean isExact
 ) {
   elementSpec(namespace, type, subtypes, name, signature, ext) and
   exists(Declaration base, Declaration d |
     base = interpretBaseDeclaration(namespace, type, name, signature) and
     (
-      d = base
+      d = base and
+      isExact = true
       or
       subtypes = true and
       (
         d.(UnboundCallable).overridesOrImplementsUnbound(base)
         or
         d = base.(UnboundValueOrRefType).getASubTypeUnbound+()
-      )
+      ) and
+      isExact = false
     )
   |
     result = interpretExt(d, ext)
@@ -500,71 +503,47 @@ string getSignature(UnboundCallable c) {
 }
 
 private predicate interpretSummary(
-  UnboundCallable c, string input, string output, string kind, string provenance, string model
+  UnboundCallable c, string input, string output, string kind, string provenance, boolean isExact,
+  string model
 ) {
   exists(
     string namespace, string type, boolean subtypes, string name, string signature, string ext
   |
     summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance,
       model) and
-    c = interpretElement(namespace, type, subtypes, name, signature, ext)
+    c = interpretElement(namespace, type, subtypes, name, signature, ext, isExact)
   )
 }
 
-predicate interpretNeutral(UnboundCallable c, string kind, string provenance) {
+predicate interpretNeutral(UnboundCallable c, string kind, string provenance, boolean isExact) {
   exists(string namespace, string type, string name, string signature |
     Extensions::neutralModel(namespace, type, name, signature, kind, provenance) and
-    c = interpretElement(namespace, type, true, name, signature, "")
+    c = interpretElement(namespace, type, true, name, signature, "", isExact)
   )
 }
 
 // adapter class for converting Mad summaries to `SummarizedCallable`s
 private class SummarizedCallableAdapter extends SummarizedCallable {
-  SummarizedCallableAdapter() {
-    exists(Provenance provenance | interpretSummary(this, _, _, _, provenance, _) |
-      not this.fromSource()
-      or
-      this.fromSource() and provenance.isManual()
-    )
-  }
-
-  private predicate relevantSummaryElementManual(
-    string input, string output, string kind, string model
-  ) {
-    exists(Provenance provenance |
-      interpretSummary(this, input, output, kind, provenance, model) and
-      provenance.isManual()
-    )
-  }
+  string input_;
+  string output_;
+  string kind;
+  Provenance p_;
+  boolean isExact_;
+  string model_;
 
-  private predicate relevantSummaryElementGenerated(
-    string input, string output, string kind, string model
-  ) {
-    exists(Provenance provenance |
-      interpretSummary(this, input, output, kind, provenance, model) and
-      provenance.isGenerated()
-    ) and
-    not exists(Provenance provenance |
-      interpretNeutral(this, "summary", provenance) and
-      provenance.isManual()
-    )
+  SummarizedCallableAdapter() {
+    interpretSummary(this, input_, output_, kind, p_, isExact_, model_)
   }
 
   override predicate propagatesFlow(
-    string input, string output, boolean preservesValue, string model
+    string input, string output, boolean preservesValue, Provenance p, boolean isExact, string model
   ) {
-    exists(string kind |
-      this.relevantSummaryElementManual(input, output, kind, model)
-      or
-      not this.relevantSummaryElementManual(_, _, _, _) and
-      this.relevantSummaryElementGenerated(input, output, kind, model)
-    |
-      if kind = "value" then preservesValue = true else preservesValue = false
-    )
-  }
-
-  override predicate hasProvenance(Provenance provenance) {
-    interpretSummary(this, _, _, _, provenance, _)
+    input = input_ and
+    output = output_ and
+    (if kind = "value" then preservesValue = true else preservesValue = false) and
+    p = p_ and
+    isExact = isExact_ and
+    model = model_
   }
 }
 

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

@@ -18,14 +18,24 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CsharpDataFlow>
 
   class SummarizedCallableBase = UnboundCallable;
 
+  predicate callableFromSource(SummarizedCallableBase c) {
+    c.fromSource() and
+    not c.getFile().isStub() and
+    not (
+      c.getFile().extractedQlTest() and
+      (
+        c.getBody() instanceof ThrowElement or
+        c.getBody().(BlockStmt).getStmt(0) instanceof ThrowElement
+      )
+    )
+  }
+
   class SourceBase = Void;
 
   class SinkBase = Void;
 
   predicate neutralElement(SummarizedCallableBase c, string kind, string provenance, boolean isExact) {
-    interpretNeutral(c, kind, provenance) and
-    // isExact is not needed for C#.
-    isExact = false
+    interpretNeutral(c, kind, provenance, isExact)
   }
 
   ArgumentPosition callbackSelfParameterPosition() { result.isDelegateSelf() }
@@ -216,7 +226,7 @@ module SourceSinkInterpretationInput implements
       string namespace, string type, boolean subtypes, string name, string signature, string ext
     |
       sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, model) and
-      e = interpretElement(namespace, type, subtypes, name, signature, ext)
+      e = interpretElement(namespace, type, subtypes, name, signature, ext, _)
     )
   }
 
@@ -227,7 +237,7 @@ module SourceSinkInterpretationInput implements
       string namespace, string type, boolean subtypes, string name, string signature, string ext
     |
       sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, model) and
-      e = interpretElement(namespace, type, subtypes, name, signature, ext)
+      e = interpretElement(namespace, type, subtypes, name, signature, ext, _)
     )
   }
 
@@ -437,13 +447,14 @@ private class SummarizedCallableWithCallback extends Public::SummarizedCallable
   SummarizedCallableWithCallback() { mayInvokeCallback(this, pos) }
 
   override predicate propagatesFlow(
-    string input, string output, boolean preservesValue, string model
+    string input, string output, boolean preservesValue, Public::Provenance provenance,
+    boolean isExact, string model
   ) {
     input = "Argument[" + pos + "]" and
     output = "Argument[" + pos + "].Parameter[delegate-self]" and
     preservesValue = true and
+    provenance = "hq-generated" and
+    isExact = true and
     model = "heuristic-callback"
   }
-
-  override predicate hasProvenance(Public::Provenance provenance) { provenance = "hq-generated" }
 }

csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll

@@ -92,22 +92,17 @@ module EntityFramework {
   abstract class EFSummarizedCallable extends SummarizedCallableImpl {
     bindingset[this]
     EFSummarizedCallable() { any() }
-
-    override predicate hasProvenance(Provenance provenance) { provenance = "manual" }
   }
 
   // see `SummarizedCallableImpl` qldoc
   private class EFSummarizedCallableAdapter extends SummarizedCallable instanceof EFSummarizedCallable
   {
     override predicate propagatesFlow(
-      string input, string output, boolean preservesValue, string model
+      string input, string output, boolean preservesValue, Provenance provenance, boolean isExact,
+      string model
     ) {
       none()
     }
-
-    override predicate hasProvenance(Provenance provenance) {
-      EFSummarizedCallable.super.hasProvenance(provenance)
-    }
   }
 
   /** The class ``Microsoft.EntityFrameworkCore.DbQuery`1`` or ``System.Data.Entity.DbQuery`1``. */
@@ -177,11 +172,13 @@ module EntityFramework {
 
     override predicate propagatesFlow(
       SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
-      string model
+      Provenance p, boolean isExact, string model
     ) {
       input = SummaryComponentStack::argument(0) and
       output = SummaryComponentStack::return() and
       preservesValue = false and
+      p = "manual" and
+      isExact = true and
       model = "RawSqlStringConstructorSummarizedCallable"
     }
   }
@@ -193,11 +190,13 @@ module EntityFramework {
 
     override predicate propagatesFlow(
       SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
-      string model
+      Provenance p, boolean isExact, string model
     ) {
       input = SummaryComponentStack::argument(0) and
       output = SummaryComponentStack::return() and
       preservesValue = false and
+      p = "manual" and
+      isExact = true and
       model = "RawSqlStringConversionSummarizedCallable"
     }
   }
@@ -459,18 +458,20 @@ module EntityFramework {
   }
 
   private class DbContextClassSetPropertySynthetic extends EFSummarizedCallable {
-    private DbContextClassSetProperty p;
+    private DbContextClassSetProperty prop;
 
-    DbContextClassSetPropertySynthetic() { this = p.getGetter() }
+    DbContextClassSetPropertySynthetic() { this = prop.getGetter() }
 
     override predicate propagatesFlow(
       SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
-      string model
+      Provenance p, boolean isExact, string model
     ) {
       exists(string name, DbContextClass c |
         preservesValue = true and
-        name = c.getSyntheticName(output, _, p) and
+        name = c.getSyntheticName(output, _, prop) and
         input = SummaryComponentStack::syntheticGlobal(name) and
+        p = "manual" and
+        isExact = true and
         model = "DbContextClassSetPropertySynthetic"
       )
     }
@@ -483,13 +484,15 @@ module EntityFramework {
 
     override predicate propagatesFlow(
       SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
-      string model
+      Provenance p, boolean isExact, string model
     ) {
       exists(string name, Property mapped |
         preservesValue = true and
         c.input(input, mapped) and
         name = c.getSyntheticNameProj(mapped) and
         output = SummaryComponentStack::syntheticGlobal(name) and
+        p = "manual" and
+        isExact = true and
         model = "DbContextSaveChanges"
       )
     }

csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll

@@ -230,7 +230,7 @@ module SummaryModelGeneratorInput implements SummaryModelGeneratorInputSig {
   }
 
   private predicate hasManualSummaryModel(Callable api) {
-    api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()) or
+    api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.hasManualModel()) or
     api = any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel())
   }
 

csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs

@@ -215,9 +215,9 @@ void M4()
             Sink(Library.GeneratedFlowWithManualNeutral(o2)); // no flow because the modelled method has a manual neutral summary model
         }
 
-        object GeneratedFlow(object o) => throw null;
+        object GeneratedFlow(object o) => null;
 
-        object GeneratedFlowArgs(object o1, object o2) => throw null;
+        object GeneratedFlowArgs(object o1, object o2) => null;
 
         static void Sink(object o) { }
     }

csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected

@@ -29,7 +29,6 @@ edges
 | Capture.cs:29:26:29:32 | access to parameter tainted : String | Capture.cs:29:17:29:22 | access to local variable sink29 : String | provenance |  |
 | Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | MaD:3 |
 | Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | MaD:4 |
-| Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | heuristic-callback |
 | Capture.cs:50:50:50:55 | sink39 : String | Capture.cs:52:23:59:13 | (...) => ... : (...) => ... [captured sink39] : String | provenance |  |
 | Capture.cs:52:23:59:13 | (...) => ... : (...) => ... [captured sink39] : String | Capture.cs:350:34:350:34 | a : (...) => ... [captured sink39] : String | provenance |  |
 | Capture.cs:55:27:58:17 | (...) => ... : (...) => ... [captured sink39] : String | Capture.cs:350:34:350:34 | a : (...) => ... [captured sink39] : String | provenance |  |
@@ -40,7 +39,6 @@ edges
 | Capture.cs:81:13:81:13 | [post] access to local function M : M [captured sink31] : String | Capture.cs:83:9:83:19 | [post] access to local function CaptureOut2 : CaptureOut2 [captured sink31] : String | provenance |  |
 | Capture.cs:83:9:83:19 | [post] access to local function CaptureOut2 : CaptureOut2 [captured sink31] : String | Capture.cs:84:15:84:20 | access to local variable sink31 | provenance |  |
 | Capture.cs:89:22:89:35 | "taint source" : String | Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | provenance |  |
-| Capture.cs:89:22:89:35 | "taint source" : String | Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | provenance | heuristic-callback |
 | Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | Capture.cs:93:15:93:20 | access to local variable sink32 | provenance |  |
 | Capture.cs:114:23:117:13 | [post] (...) => ... : (...) => ... [captured sink40] : String | Capture.cs:123:9:123:33 | [post] access to local function CaptureOutMultipleLambdas : CaptureOutMultipleLambdas [captured sink40] : String | provenance |  |
 | Capture.cs:116:26:116:39 | "taint source" : String | Capture.cs:352:9:352:9 | [post] access to parameter a : (...) => ... [captured sink40] : String | provenance |  |
@@ -62,10 +60,8 @@ edges
 | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | Capture.cs:156:15:156:20 | access to local variable sink35 | provenance |  |
 | Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | MaD:3 |
 | Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | MaD:4 |
-| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | heuristic-callback |
 | Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | MaD:3 |
 | Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | MaD:4 |
-| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | heuristic-callback |
 | Capture.cs:162:13:162:18 | access to local variable sink36 : String | Capture.cs:163:15:163:20 | access to local variable sink36 | provenance |  |
 | Capture.cs:162:22:162:36 | access to local function CaptureThrough4 : CaptureThrough4 [captured tainted] : String | Capture.cs:160:20:160:26 | access to parameter tainted : String | provenance |  |
 | Capture.cs:162:22:162:36 | access to local function CaptureThrough4 : CaptureThrough4 [captured tainted] : String | Capture.cs:162:22:162:38 | call to local function CaptureThrough4 : String | provenance |  |
@@ -94,10 +90,8 @@ edges
 | Capture.cs:228:17:228:30 | "taint source" : String | Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | provenance |  |
 | Capture.cs:228:17:228:30 | "taint source" : String | Capture.cs:234:15:234:15 | access to local variable x | provenance |  |
 | Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | Capture.cs:231:19:231:19 | access to local variable x | provenance | MaD:1 |
-| Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | Capture.cs:231:19:231:19 | access to local variable x | provenance | heuristic-callback |
 | Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | Capture.cs:234:15:234:15 | access to local variable x | provenance |  |
 | Capture.cs:232:17:232:30 | "taint source" : String | Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | provenance |  |
-| Capture.cs:232:17:232:30 | "taint source" : String | Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | provenance | heuristic-callback |
 | Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | Capture.cs:249:9:249:9 | access to local variable a : Action [captured c, field Field] : String | provenance |  |
 | Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | Capture.cs:251:15:251:15 | access to local variable c : Capture [field Field] : String | provenance |  |
 | Capture.cs:242:19:242:32 | "taint source" : String | Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | provenance |  |