Shared: Provenance-based filtering of flow summaries

cpp/ql/lib/semmle/code/cpp/dataflow/ExternalFlow.qll

@@ -1058,40 +1058,22 @@ private predicate interpretSummary(
 
 // adapter class for converting Mad summaries to `SummarizedCallable`s
 private class SummarizedCallableAdapter extends SummarizedCallable {
-  SummarizedCallableAdapter() { interpretSummary(this, _, _, _, _, _) }
+  string input_;
+  string output_;
+  string kind;
+  Provenance p_;
+  string model_;
 
-  private predicate relevantSummaryElementManual(
-    string input, string output, string kind, string model
-  ) {
-    exists(Provenance provenance |
-      interpretSummary(this, input, output, kind, provenance, model) and
-      provenance.isManual()
-    )
-  }
-
-  private predicate relevantSummaryElementGenerated(
-    string input, string output, string kind, string model
-  ) {
-    exists(Provenance provenance |
-      interpretSummary(this, input, output, kind, provenance, model) and
-      provenance.isGenerated()
-    )
-  }
+  SummarizedCallableAdapter() { interpretSummary(this, input_, output_, kind, p_, 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 = true and
+    model = model_
   }
 }

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

@@ -20,6 +20,8 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
 
   class SinkBase = Void;
 
+  predicate callableFromSource(SummarizedCallableBase c) { exists(c.getBlock()) }
+
   ArgumentPosition callbackSelfParameterPosition() { result = TDirectPosition(-1) }
 
   ReturnKind getStandardReturnValueKind() { result = getReturnValueKind("") }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -1144,7 +1144,7 @@ private newtype TDataFlowCall =
   }
 
 private predicate summarizedCallableIsManual(SummarizedCallable sc) {
-  sc.asSummarizedCallable().applyManualModel()
+  sc.asSummarizedCallable().hasManualModel()
 }
 
 /**

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

@@ -310,7 +310,7 @@ private module SummaryModelGeneratorInput implements SummaryModelGeneratorInputS
   }
 
   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())
   }
 

cpp/ql/test/library-tests/dataflow/external-models/steps.expected

@@ -3,6 +3,4 @@
 | test.cpp:21:27:21:27 | x | test.cpp:21:10:21:25 | call to ymlStepGenerated |
 | test.cpp:25:35:25:35 | x | test.cpp:25:11:25:33 | call to ymlStepManual_with_body |
 | test.cpp:28:35:28:35 | 0 | test.cpp:28:11:28:33 | call to ymlStepManual_with_body |
-| test.cpp:32:38:32:38 | 0 | test.cpp:32:11:32:36 | call to ymlStepGenerated_with_body |
-| test.cpp:35:38:35:38 | x | test.cpp:35:11:35:36 | call to ymlStepGenerated_with_body |
 | windows.cpp:27:36:27:38 | *cmd | windows.cpp:27:17:27:34 | **call to CommandLineToArgvA |

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;
+}
+
+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
@@ -1119,6 +1118,7 @@ private module Cached {
       p = c.asCallable(_).(CallableUsedInSource).getAParameter()
     } or
     TInstanceParameterNode(InstanceCallable c, Location l) {
+      c = any(DataFlowCallable dfc).asCallable(l) and
       c instanceof CallableUsedInSource and
       l = c.getARelevantLocation()
     } or

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

@@ -380,20 +380,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)
@@ -586,71 +589,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, _)
     )
   }
 
@@ -238,7 +248,7 @@ module SourceSinkInterpretationInput implements
       string namespace, string type, boolean subtypes, string name, string signature, string ext
     |
       barrierModel(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, _)
     )
   }
 
@@ -251,7 +261,7 @@ module SourceSinkInterpretationInput implements
     |
       barrierGuardModel(namespace, type, subtypes, name, signature, ext, input, acceptingvalue,
         kind, provenance, model) and
-      e = interpretElement(namespace, type, subtypes, name, signature, ext)
+      e = interpretElement(namespace, type, subtypes, name, signature, ext, _)
     )
   }
 
@@ -448,13 +458,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" }
 }