Merge pull request #16520 from michaelnebel/csharp/fixsummarizedcallabledataflow

C#: Make the flow summary filtering in the adapter.

Author: michaelnebel

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

@@ -24,19 +24,6 @@ newtype TReturnKind =
   TOutReturnKind(int i) { i = any(Parameter p | p.isOut()).getPosition() } or
   TRefReturnKind(int i) { i = any(Parameter p | p.isRef()).getPosition() }
 
-/**
- * A summarized callable where the summary should be used for dataflow analysis.
- */
-class DataFlowSummarizedCallable instanceof FlowSummary::SummarizedCallable {
-  DataFlowSummarizedCallable() {
-    not this.hasBody()
-    or
-    this.hasBody() and not this.applyGeneratedModel()
-  }
-
-  string toString() { result = super.toString() }
-}
-
 cached
 private module Cached {
   /**
@@ -47,7 +34,7 @@ private module Cached {
   cached
   newtype TDataFlowCallable =
     TCallable(Callable c) { c.isUnboundDeclaration() } or
-    TSummarizedCallable(DataFlowSummarizedCallable sc) or
+    TSummarizedCallable(FlowSummary::SummarizedCallable sc) or
     TFieldOrPropertyCallable(FieldOrProperty f) or
     TCapturedVariableCallable(LocalScopeVariable v) { v.isCaptured() }
 

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

@@ -1181,8 +1181,7 @@ private module Cached {
     or
     // Simple flow through library code is included in the exposed local
     // step relation, even though flow is technically inter-procedural
-    FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo,
-      any(DataFlowSummarizedCallable sc))
+    FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo, _)
   }
 
   cached

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

@@ -554,7 +554,13 @@ private predicate interpretNeutral(UnboundCallable c, string kind, string proven
 
 // adapter class for converting Mad summaries to `SummarizedCallable`s
 private class SummarizedCallableAdapter extends SummarizedCallable {
-  SummarizedCallableAdapter() { interpretSummary(this, _, _, _, _, _) }
+  SummarizedCallableAdapter() {
+    exists(Provenance provenance | interpretSummary(this, _, _, _, provenance, _) |
+      not this.hasBody()
+      or
+      this.hasBody() and provenance.isManual()
+    )
+  }
 
   private predicate relevantSummaryElementManual(
     string input, string output, string kind, string model

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

@@ -119,22 +119,19 @@ private module Cached {
     (
       // Simple flow through library code is included in the exposed local
       // step relation, even though flow is technically inter-procedural
-      FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo,
-        any(DataFlowSummarizedCallable sc))
+      FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo, _)
       or
       // Taint collection by adding a tainted element
       exists(DataFlow::ElementContent c |
         storeStep(nodeFrom, c, nodeTo)
         or
-        FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo,
-          any(DataFlowSummarizedCallable sc))
+        FlowSummaryImpl::Private::Steps::summarySetterStep(nodeFrom, c, nodeTo, _)
       )
       or
       exists(DataFlow::Content c |
         readStep(nodeFrom, c, nodeTo)
         or
-        FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo,
-          any(DataFlowSummarizedCallable sc))
+        FlowSummaryImpl::Private::Steps::summaryGetterStep(nodeFrom, c, nodeTo, _)
       |
         // Taint members
         c = any(TaintedMember m).(FieldOrProperty).getContent()

csharp/ql/src/Language Abuse/ForeachCapture.ql

@@ -77,8 +77,7 @@ Element getAssignmentTarget(Expr e) {
 Element getCollectionAssignmentTarget(Expr e) {
   // Store into collection via method
   exists(DataFlowPrivate::PostUpdateNode postNode |
-    FlowSummaryImpl::Private::Steps::summarySetterStep(DataFlow::exprNode(e), _, postNode,
-      any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
+    FlowSummaryImpl::Private::Steps::summarySetterStep(DataFlow::exprNode(e), _, postNode, _) and
     result.(Variable).getAnAccess() = postNode.getPreUpdateNode().asExpr()
   )
   or

csharp/ql/test/library-tests/dataflow/external-models/steps.ql

@@ -25,21 +25,17 @@ private class StepArgQualGenerated extends Method {
 query predicate summaryThroughStep(
   DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue
 ) {
-  FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2,
-    any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
+  FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2, _) and
   preservesValue = true
   or
-  FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2,
-    any(DataFlowDispatch::DataFlowSummarizedCallable sc)) and
+  FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2, _) and
   preservesValue = false
 }
 
 query predicate summaryGetterStep(DataFlow::Node arg, DataFlow::Node out, Content c) {
-  FlowSummaryImpl::Private::Steps::summaryGetterStep(arg, c, out,
-    any(DataFlowDispatch::DataFlowSummarizedCallable sc))
+  FlowSummaryImpl::Private::Steps::summaryGetterStep(arg, c, out, _)
 }
 
 query predicate summarySetterStep(DataFlow::Node arg, DataFlow::Node out, Content c) {
-  FlowSummaryImpl::Private::Steps::summarySetterStep(arg, c, out,
-    any(DataFlowDispatch::DataFlowSummarizedCallable sc))
+  FlowSummaryImpl::Private::Steps::summarySetterStep(arg, c, out, _)
 }