From fe854812ec7b61892332bcce72a0ddd86cffea06 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Fri, 11 Oct 2024 12:09:49 +0200
Subject: [PATCH 1/7] C#: Add read and store steps for delegate calls.

---
 .../dataflow/internal/DataFlowPrivate.qll     | 88 ++++++++++++++++++-
 .../dataflow/internal/DataFlowPublic.qll      | 70 +++++++++++++++
 .../modelgenerator/internal/CaptureModels.qll | 12 ++-
 .../internal/ModelGeneratorImpl.qll           | 17 +++-
 4 files changed, 181 insertions(+), 6 deletions(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
index 581e85182a81..8660bb871e13 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -13,6 +13,7 @@ private import semmle.code.csharp.Unification
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.frameworks.EntityFramework
+private import semmle.code.csharp.frameworks.system.linq.Expressions
 private import semmle.code.csharp.frameworks.NHibernate
 private import semmle.code.csharp.frameworks.Razor
 private import semmle.code.csharp.frameworks.system.Collections
@@ -1146,7 +1147,18 @@ private module Cached {
     TPrimaryConstructorParameterContent(Parameter p) {
       p.getCallable() instanceof PrimaryConstructor
     } or
-    TCapturedVariableContent(VariableCapture::CapturedVariable v)
+    TCapturedVariableContent(VariableCapture::CapturedVariable v) or
+    TDelegateCallArgumentContent(Parameter p, int i) {
+      i =
+        [0 .. p.getType()
+                .getUnboundDeclaration()
+                .(SystemLinqExpressions::DelegateExtType)
+                .getDelegateType()
+                .getNumberOfParameters() - 1]
+    } or
+    TDelegateCallReturnContent(Parameter p) {
+      p.getType().getUnboundDeclaration() instanceof SystemLinqExpressions::DelegateExtType
+    }
 
   cached
   newtype TContentSet =
@@ -1162,7 +1174,13 @@ private module Cached {
     TPrimaryConstructorParameterApproxContent(string firstChar) {
       firstChar = approximatePrimaryConstructorParameterContent(_)
     } or
-    TCapturedVariableContentApprox(VariableCapture::CapturedVariable v)
+    TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or
+    TDelegateCallArgumentApproxContent(string firstChar) {
+      firstChar = approximateDelegateCallArgumentContent(_)
+    } or
+    TDelegateCallReturnApproxContent(string firstChar) {
+      firstChar = approximateDelegateCallReturnContent(_)
+    }
 
   pragma[nomagic]
   private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) {
@@ -2273,6 +2291,22 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
   )
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to
+ * the content set `c` of a delegate call.
+ *
+ * If there is a delegate call f(x), then we store "x" on "f"
+ * using a delegate parameter content set.
+ */
+private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) {
+  exists(DelegateCall call, Parameter p, int i |
+    node1.asExpr() = call.getArgument(i) and
+    node2.(PostUpdateNode).getPreUpdateNode().asExpr() = call.getExpr() and
+    call.getExpr() = p.getAnAccess() and
+    c.isDelegateCallArgument(p, i)
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to
  * content `c`.
@@ -2305,6 +2339,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  storeStepDelegateCall(node1, c, node2)
 }
 
 private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -2425,6 +2461,22 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
   VariableCapture::readStep(node1, c, node2)
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to
+ * the content set `c` of a delegate call.
+ *
+ * If there is a delegate call f(x), then we read the result of the delegate
+ * call.
+ */
+private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) {
+  exists(DelegateCall call, Parameter p |
+    node1.asExpr() = call.getExpr() and
+    node2.asExpr() = call and
+    call.getExpr() = p.getAnAccess() and
+    c.isDelegateCallReturn(p)
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of content `c`.
  */
@@ -2443,6 +2495,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  readStepDelegateCall(node1, c, node2)
 }
 
 private predicate clearsCont(Node n, Content c) {
@@ -3037,6 +3091,16 @@ class ContentApprox extends TContentApprox {
     exists(VariableCapture::CapturedVariable v |
       this = TCapturedVariableContentApprox(v) and result = "captured " + v
     )
+    or
+    exists(string firstChar |
+      this = TDelegateCallArgumentApproxContent(firstChar) and
+      result = "approximated delegate call argument " + firstChar
+    )
+    or
+    exists(string firstChar |
+      this = TDelegateCallReturnApproxContent(firstChar) and
+      result = "approximated delegate call return " + firstChar
+    )
   }
 }
 
@@ -3058,6 +3122,22 @@ private string approximatePrimaryConstructorParameterContent(PrimaryConstructorP
   result = pc.getParameter().getName().prefix(1)
 }
 
+private string getApproximateParameterName(Parameter p) {
+  exists(string name | name = p.getName() |
+    name = "" and result = ""
+    or
+    result = name.prefix(1)
+  )
+}
+
+private string approximateDelegateCallArgumentContent(DelegateCallArgumentContent dc) {
+  result = getApproximateParameterName(dc.getParameter())
+}
+
+private string approximateDelegateCallReturnContent(DelegateCallReturnContent dc) {
+  result = getApproximateParameterName(dc.getParameter())
+}
+
 /** Gets an approximated value for content `c`. */
 pragma[nomagic]
 ContentApprox getContentApprox(Content c) {
@@ -3073,6 +3153,10 @@ ContentApprox getContentApprox(Content c) {
     TPrimaryConstructorParameterApproxContent(approximatePrimaryConstructorParameterContent(c))
   or
   result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c))
+  or
+  result = TDelegateCallArgumentApproxContent(approximateDelegateCallArgumentContent(c))
+  or
+  result = TDelegateCallReturnApproxContent(approximateDelegateCallReturnContent(c))
 }
 
 /**
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
index 6717f8659eab..c0f0f3a08fd8 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -3,6 +3,7 @@ private import DataFlowDispatch
 private import DataFlowPrivate
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.Unification
+private import semmle.code.csharp.frameworks.system.linq.Expressions
 
 /**
  * An element, viewed as a node in a data flow graph. Either an expression
@@ -238,6 +239,61 @@ class PropertyContent extends Content, TPropertyContent {
   override Location getLocation() { result = p.getLocation() }
 }
 
+/**
+ * A reference to the index of an argument of a delegate call
+ * (where the delegate is a parameter)
+ */
+class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent {
+  private Parameter p;
+  private int i;
+
+  DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(p, i) }
+
+  /** Gets the underlying parameter. */
+  Parameter getParameter() { result = p }
+
+  /**
+   * Gets the type of the `i`th parameter of the underlying parameter `p`
+   * (which is of delegate type).
+   */
+  Type getType() {
+    result =
+      p.getType()
+          .(SystemLinqExpressions::DelegateExtType)
+          .getDelegateType()
+          .getParameter(i)
+          .getType()
+  }
+
+  override string toString() { result = "delegate parameter " + p.getName() + " at position " + i }
+
+  override Location getLocation() { result = p.getLocation() }
+}
+
+/**
+ * A reference to the return of a delegate call
+ * (where the delegate is a parameter)
+ */
+class DelegateCallReturnContent extends Content, TDelegateCallReturnContent {
+  private Parameter p;
+
+  DelegateCallReturnContent() { this = TDelegateCallReturnContent(p) }
+
+  /** Gets the underlying parameter. */
+  Parameter getParameter() { result = p }
+
+  /**
+   * Gets the return type of the underlying parameter `p` (which is of delegate type).
+   */
+  Type getType() {
+    result = p.getType().(SystemLinqExpressions::DelegateExtType).getDelegateType().getReturnType()
+  }
+
+  override string toString() { result = "delegate parameter " + p.getName() + " result" }
+
+  override Location getLocation() { result = p.getLocation() }
+}
+
 /**
  * A reference to a synthetic field corresponding to a
  * primary constructor parameter.
@@ -299,6 +355,20 @@ class ContentSet extends TContentSet {
    */
   predicate isProperty(Property p) { this = TPropertyContentSet(p) }
 
+  /**
+   * Holds if this content set represents the `i`th argument of
+   * the parameter `p` of delegate type in a delegate call.
+   */
+  predicate isDelegateCallArgument(Parameter p, int i) {
+    this.isSingleton(TDelegateCallArgumentContent(p, i))
+  }
+
+  /**
+   * Holds if this content set represents the return of a delegate call
+   * of parameter `p` (which is of delegate type).
+   */
+  predicate isDelegateCallReturn(Parameter p) { this.isSingleton(TDelegateCallReturnContent(p)) }
+
   /** Holds if this content set represents the field `f`. */
   predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }
 
diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
index fb2bfafbf258..e815b38ef9e3 100644
--- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
+++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -101,7 +101,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel())
   }
 
-  predicate isUninterestingForDataFlowModels(Callable api) { isHigherOrder(api) }
+  predicate isUninterestingForDataFlowModels(Callable api) { none() }
+
+  predicate isUninterestingForHeuristicDataFlowModels(Callable api) { isHigherOrder(api) }
 
   class SourceOrSinkTargetApi extends Callable {
     SourceOrSinkTargetApi() { relevant(this) }
@@ -175,7 +177,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
    */
   private CS::Type getUnderlyingContType(DataFlow::Content c) {
     result = c.(DataFlow::FieldContent).getField().getType() or
-    result = c.(DataFlow::SyntheticFieldContent).getField().getType()
+    result = c.(DataFlow::SyntheticFieldContent).getField().getType() or
+    result = c.(DataFlow::DelegateCallArgumentContent).getType() or
+    result = c.(DataFlow::DelegateCallReturnContent).getType()
   }
 
   Type getUnderlyingContentType(DataFlow::ContentSet c) {
@@ -342,6 +346,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     or
     c.isElement() and
     result = "Element"
+    or
+    exists(int i | c.isDelegateCallArgument(_, i) and result = "Parameter[" + i + "]")
+    or
+    c.isDelegateCallReturn(_) and result = "ReturnValue"
   }
 
   predicate partialModel = ExternalFlow::partialModel/6;
diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
index d38b83bcf9c4..b59d9d8bcb82 100644
--- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
+++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
@@ -223,6 +223,15 @@ signature module ModelGeneratorInputSig<LocationSig Location, InputSig<Location>
    */
   predicate isUninterestingForDataFlowModels(Callable api);
 
+  /**
+   * Holds if it is irrelevant to generate models for `api` based on the heuristic
+   * (non-content) flow analysis.
+   *
+   * This serves as an extra filter for the `relevant`
+   * and `isUninterestingForDataFlowModels` predicates.
+   */
+  predicate isUninterestingForHeuristicDataFlowModels(Callable api);
+
   /**
    * Holds if `namespace`, `type`, `extensible`, `name` and `parameters` are string representations
    * for the corresponding MaD columns for `api`.
@@ -300,7 +309,7 @@ module MakeModelGenerator<
     }
   }
 
-  string getOutput(ReturnNodeExt node) {
+  private string getOutput(ReturnNodeExt node) {
     result = PrintReturnNodeExt<paramReturnNodeAsOutput/2>::getOutput(node)
   }
 
@@ -432,7 +441,11 @@ module MakeModelGenerator<
 
     predicate isSource(DataFlow::Node source, FlowState state) {
       source instanceof DataFlow::ParameterNode and
-      source.(NodeExtended).getEnclosingCallable() instanceof DataFlowSummaryTargetApi and
+      exists(Callable c |
+        c = source.(NodeExtended).getEnclosingCallable() and
+        c instanceof DataFlowSummaryTargetApi and
+        not isUninterestingForHeuristicDataFlowModels(c)
+      ) and
       state.(TaintRead).getStep() = 0
     }
 

From 395cababb36bbb86a6999a07c55ee680c4bea186 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Fri, 11 Oct 2024 12:10:12 +0200
Subject: [PATCH 2/7] C#: Add some model generator examples for higher order
 methods.

---
 .../modelgenerator/dataflow/Summaries.cs      | 45 +++++++++++++++++--
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
index 473144358f47..f7c583330ef8 100644
--- a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
+++ b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
@@ -497,18 +497,55 @@ public Type M6(Type t)
     }
 }
 
-// No models as higher order methods are excluded
-// from model generation.
+// Methods in this class are "neutral" with respect to the heuristic model generation, but
+// the content based model generation is able to produce flow summaries for them.
 public class HigherOrderParameters
 {
+    // neutral=Models;HigherOrderParameters;M1;(System.String,System.Func<System.String,System.String>);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;M1;(System.String,System.Func<System.String,System.String>);;Argument[0];ReturnValue;value;dfc-generated
     public string M1(string s, Func<string, string> map)
     {
         return s;
     }
 
-    public object M2(Func<object, object> map, object o)
+    // neutral=Models;HigherOrderParameters;Map;(System.Func<System.Object,System.Object>,System.Object);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[0].ReturnValue;ReturnValue;value;dfc-generated
+    public object Map(Func<object, object> f, object o)
     {
-        return map(o);
+        return f(o);
+    }
+
+    // neutral=Models;HigherOrderParameters;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[0];Argument[1].Parameter[1];value;dfc-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[1].ReturnValue;ReturnValue;value;dfc-generated
+    public object Map2(object o, Func<object, object, object> f)
+    {
+        var x = f(null, o);
+        return x;
+    }
+
+    // neutral=Models;HigherOrderParameters;Apply;(System.Action<System.Object>,System.Object);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Action<System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
+    public void Apply(Action<object> a, object o)
+    {
+        a(o);
+    }
+}
+
+public static class HigherOrderExtensionMethods
+{
+    // neutral=Models;HigherOrderExtensionMethods;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);summary;df-generated
+    // contentbased-summary=Models;HigherOrderExtensionMethods;false;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);;Argument[0].Element;Argument[1].Parameter[0];value;dfc-generated
+    // contentbased-summary=Models;HigherOrderExtensionMethods;false;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);;Argument[1].ReturnValue;ReturnValue.Element;value;dfc-generated
+    public static IEnumerable<TResult> Select<TSource, TResult>(
+            this IEnumerable<TSource> source,
+            Func<TSource, TResult> selector)
+    {
+        foreach (var item in source)
+        {
+            yield return selector(item);
+        }
     }
 }
 

From a86cd181a6197d3473f128c3e5271e9cb31353a5 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Thu, 24 Oct 2024 10:48:27 +0200
Subject: [PATCH 3/7] Java: Make language specific modifications.

---
 java/ql/src/utils/modelgenerator/internal/CaptureModels.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
index 6239b535c986..32d3f779cca6 100644
--- a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
+++ b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -88,6 +88,8 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
     api.getDeclaringType() instanceof J::Interface and not exists(api.getBody())
   }
 
+  predicate isUninterestingForHeuristicDataFlowModels(Callable api) { none() }
+
   class SourceOrSinkTargetApi extends Callable {
     SourceOrSinkTargetApi() { relevant(this) }
   }

From 5c389355d0208c261445bb2f774bd17241dad14c Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Fri, 25 Oct 2024 13:02:21 +0200
Subject: [PATCH 4/7] C#: Simplify delegate read and store steps (remove
 dependency on parameter).

---
 .../dataflow/internal/DataFlowPrivate.qll     | 69 +++++--------------
 .../dataflow/internal/DataFlowPublic.qll      | 59 ++++------------
 .../modelgenerator/internal/CaptureModels.qll | 17 +++--
 3 files changed, 42 insertions(+), 103 deletions(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
index 8660bb871e13..ff3531e677de 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -1148,17 +1148,10 @@ private module Cached {
       p.getCallable() instanceof PrimaryConstructor
     } or
     TCapturedVariableContent(VariableCapture::CapturedVariable v) or
-    TDelegateCallArgumentContent(Parameter p, int i) {
-      i =
-        [0 .. p.getType()
-                .getUnboundDeclaration()
-                .(SystemLinqExpressions::DelegateExtType)
-                .getDelegateType()
-                .getNumberOfParameters() - 1]
+    TDelegateCallArgumentContent(int i) {
+      i = [0 .. max(int j | j = any(DelegateCall dc).getNumberOfArguments())]
     } or
-    TDelegateCallReturnContent(Parameter p) {
-      p.getType().getUnboundDeclaration() instanceof SystemLinqExpressions::DelegateExtType
-    }
+    TDelegateCallReturnContent()
 
   cached
   newtype TContentSet =
@@ -1175,12 +1168,8 @@ private module Cached {
       firstChar = approximatePrimaryConstructorParameterContent(_)
     } or
     TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or
-    TDelegateCallArgumentApproxContent(string firstChar) {
-      firstChar = approximateDelegateCallArgumentContent(_)
-    } or
-    TDelegateCallReturnApproxContent(string firstChar) {
-      firstChar = approximateDelegateCallReturnContent(_)
-    }
+    TDelegateCallArgumentApproxContent() or
+    TDelegateCallReturnApproxContent()
 
   pragma[nomagic]
   private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) {
@@ -2296,14 +2285,13 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
  * the content set `c` of a delegate call.
  *
  * If there is a delegate call f(x), then we store "x" on "f"
- * using a delegate parameter content set.
+ * using a delegate argument content set.
  */
 private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) {
-  exists(DelegateCall call, Parameter p, int i |
+  exists(DelegateCall call, int i |
     node1.asExpr() = call.getArgument(i) and
     node2.(PostUpdateNode).getPreUpdateNode().asExpr() = call.getExpr() and
-    call.getExpr() = p.getAnAccess() and
-    c.isDelegateCallArgument(p, i)
+    c.isDelegateCallArgument(i)
   )
 }
 
@@ -2465,15 +2453,14 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
  * Holds if data can flow from `node1` to `node2` via an assignment to
  * the content set `c` of a delegate call.
  *
- * If there is a delegate call f(x), then we read the result of the delegate
+ * If there is a delegate call f(x), then we read the return of the delegate
  * call.
  */
 private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) {
-  exists(DelegateCall call, Parameter p |
+  exists(DelegateCall call |
     node1.asExpr() = call.getExpr() and
     node2.asExpr() = call and
-    call.getExpr() = p.getAnAccess() and
-    c.isDelegateCallReturn(p)
+    c.isDelegateCallReturn()
   )
 }
 
@@ -3092,15 +3079,11 @@ class ContentApprox extends TContentApprox {
       this = TCapturedVariableContentApprox(v) and result = "captured " + v
     )
     or
-    exists(string firstChar |
-      this = TDelegateCallArgumentApproxContent(firstChar) and
-      result = "approximated delegate call argument " + firstChar
-    )
+    this = TDelegateCallArgumentApproxContent() and
+    result = "approximated delegate call argument"
     or
-    exists(string firstChar |
-      this = TDelegateCallReturnApproxContent(firstChar) and
-      result = "approximated delegate call return " + firstChar
-    )
+    this = TDelegateCallReturnApproxContent() and
+    result = "approximated delegate call return"
   }
 }
 
@@ -3122,22 +3105,6 @@ private string approximatePrimaryConstructorParameterContent(PrimaryConstructorP
   result = pc.getParameter().getName().prefix(1)
 }
 
-private string getApproximateParameterName(Parameter p) {
-  exists(string name | name = p.getName() |
-    name = "" and result = ""
-    or
-    result = name.prefix(1)
-  )
-}
-
-private string approximateDelegateCallArgumentContent(DelegateCallArgumentContent dc) {
-  result = getApproximateParameterName(dc.getParameter())
-}
-
-private string approximateDelegateCallReturnContent(DelegateCallReturnContent dc) {
-  result = getApproximateParameterName(dc.getParameter())
-}
-
 /** Gets an approximated value for content `c`. */
 pragma[nomagic]
 ContentApprox getContentApprox(Content c) {
@@ -3154,9 +3121,11 @@ ContentApprox getContentApprox(Content c) {
   or
   result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c))
   or
-  result = TDelegateCallArgumentApproxContent(approximateDelegateCallArgumentContent(c))
+  c instanceof DelegateCallArgumentContent and
+  result = TDelegateCallArgumentApproxContent()
   or
-  result = TDelegateCallReturnApproxContent(approximateDelegateCallReturnContent(c))
+  c instanceof DelegateCallReturnContent and
+  result = TDelegateCallReturnApproxContent()
 }
 
 /**
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
index c0f0f3a08fd8..c5460652746b 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
@@ -240,58 +240,27 @@ class PropertyContent extends Content, TPropertyContent {
 }
 
 /**
- * A reference to the index of an argument of a delegate call
- * (where the delegate is a parameter)
+ * A reference to the index of an argument of a delegate call.
  */
 class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent {
-  private Parameter p;
   private int i;
 
-  DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(p, i) }
+  DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(i) }
 
-  /** Gets the underlying parameter. */
-  Parameter getParameter() { result = p }
+  override string toString() { result = "delegate argument at position " + i }
 
-  /**
-   * Gets the type of the `i`th parameter of the underlying parameter `p`
-   * (which is of delegate type).
-   */
-  Type getType() {
-    result =
-      p.getType()
-          .(SystemLinqExpressions::DelegateExtType)
-          .getDelegateType()
-          .getParameter(i)
-          .getType()
-  }
-
-  override string toString() { result = "delegate parameter " + p.getName() + " at position " + i }
-
-  override Location getLocation() { result = p.getLocation() }
+  override Location getLocation() { result instanceof EmptyLocation }
 }
 
 /**
- * A reference to the return of a delegate call
- * (where the delegate is a parameter)
+ * A reference to the return of a delegate call.
  */
 class DelegateCallReturnContent extends Content, TDelegateCallReturnContent {
-  private Parameter p;
+  DelegateCallReturnContent() { this = TDelegateCallReturnContent() }
 
-  DelegateCallReturnContent() { this = TDelegateCallReturnContent(p) }
+  override string toString() { result = "delegate return" }
 
-  /** Gets the underlying parameter. */
-  Parameter getParameter() { result = p }
-
-  /**
-   * Gets the return type of the underlying parameter `p` (which is of delegate type).
-   */
-  Type getType() {
-    result = p.getType().(SystemLinqExpressions::DelegateExtType).getDelegateType().getReturnType()
-  }
-
-  override string toString() { result = "delegate parameter " + p.getName() + " result" }
-
-  override Location getLocation() { result = p.getLocation() }
+  override Location getLocation() { result instanceof EmptyLocation }
 }
 
 /**
@@ -356,18 +325,14 @@ class ContentSet extends TContentSet {
   predicate isProperty(Property p) { this = TPropertyContentSet(p) }
 
   /**
-   * Holds if this content set represents the `i`th argument of
-   * the parameter `p` of delegate type in a delegate call.
+   * Holds if this content set represents the `i`th argument of a delegate call.
    */
-  predicate isDelegateCallArgument(Parameter p, int i) {
-    this.isSingleton(TDelegateCallArgumentContent(p, i))
-  }
+  predicate isDelegateCallArgument(int i) { this.isSingleton(TDelegateCallArgumentContent(i)) }
 
   /**
-   * Holds if this content set represents the return of a delegate call
-   * of parameter `p` (which is of delegate type).
+   * Holds if this content set represents the return of a delegate call.
    */
-  predicate isDelegateCallReturn(Parameter p) { this.isSingleton(TDelegateCallReturnContent(p)) }
+  predicate isDelegateCallReturn() { this.isSingleton(TDelegateCallReturnContent()) }
 
   /** Holds if this content set represents the field `f`. */
   predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }
diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
index e815b38ef9e3..7f5598f2506d 100644
--- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
+++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -176,10 +176,15 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
    * Gets the underlying type of the content `c`.
    */
   private CS::Type getUnderlyingContType(DataFlow::Content c) {
-    result = c.(DataFlow::FieldContent).getField().getType() or
-    result = c.(DataFlow::SyntheticFieldContent).getField().getType() or
-    result = c.(DataFlow::DelegateCallArgumentContent).getType() or
-    result = c.(DataFlow::DelegateCallReturnContent).getType()
+    result = c.(DataFlow::FieldContent).getField().getType()
+    or
+    result = c.(DataFlow::SyntheticFieldContent).getField().getType()
+    or
+    // Use System.Object as the type of delegate arguments and returns as the content doesn't
+    // contain any type information.
+    c instanceof DataFlow::DelegateCallArgumentContent and result instanceof ObjectType
+    or
+    c instanceof DataFlow::DelegateCallReturnContent and result instanceof ObjectType
   }
 
   Type getUnderlyingContentType(DataFlow::ContentSet c) {
@@ -347,9 +352,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     c.isElement() and
     result = "Element"
     or
-    exists(int i | c.isDelegateCallArgument(_, i) and result = "Parameter[" + i + "]")
+    exists(int i | c.isDelegateCallArgument(i) and result = "Parameter[" + i + "]")
     or
-    c.isDelegateCallReturn(_) and result = "ReturnValue"
+    c.isDelegateCallReturn() and result = "ReturnValue"
   }
 
   predicate partialModel = ExternalFlow::partialModel/6;

From 55cfbccd43652d978e0991b178e0dc0cc9c72e05 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Tue, 29 Oct 2024 09:26:01 +0100
Subject: [PATCH 5/7] C#/Java: Exclude summaries using callbacks in fields,
 properties and synthetic fields.

---
 .../modelgenerator/internal/CaptureModels.qll |  4 ++++
 .../modelgenerator/dataflow/Summaries.cs      |  9 ++++++++
 .../modelgenerator/internal/CaptureModels.qll |  2 ++
 .../internal/ModelGeneratorImpl.qll           | 21 +++++++++++++++++++
 4 files changed, 36 insertions(+)

diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
index 7f5598f2506d..aa456fe2c790 100644
--- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
+++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -318,6 +318,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     c.isField(_) or c.isSyntheticField(_) or c.isProperty(_)
   }
 
+  predicate isCallback(DataFlow::ContentSet c) {
+    c.isDelegateCallArgument(_) or c.isDelegateCallReturn()
+  }
+
   string getSyntheticName(DataFlow::ContentSet c) {
     exists(CS::Field f |
       not f.isEffectivelyPublic() and
diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
index f7c583330ef8..f1b8272d6b94 100644
--- a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
+++ b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
@@ -62,6 +62,15 @@ public string ReturnField()
     {
         return tainted;
     }
+
+    public Func<object, object> MyFunction;
+    // summary=Models;BasicFlow;false;MapMyFunction;(System.Object);;Argument[0];Argument[this];taint;df-generated
+    // summary=Models;BasicFlow;false;MapMyFunction;(System.Object);;Argument[this];ReturnValue;taint;df-generated
+    // No content based flow as MaD doesn't support callback logic in fields and properties.
+    public object MapMyFunction(object o)
+    {
+        return MyFunction(o);
+    }
 }
 
 public class CollectionFlow
diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
index 32d3f779cca6..3e8859be9326 100644
--- a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
+++ b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -254,6 +254,8 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
     c instanceof DataFlowUtil::SyntheticFieldContent
   }
 
+  predicate isCallback(DataFlow::ContentSet c) { none() }
+
   string getSyntheticName(DataFlow::ContentSet c) {
     exists(Field f |
       not f.isPublic() and
diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
index b59d9d8bcb82..bcd1b8a19c5c 100644
--- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
+++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
@@ -206,6 +206,11 @@ signature module ModelGeneratorInputSig<LocationSig Location, InputSig<Location>
    */
   predicate isField(Lang::ContentSet c);
 
+  /**
+   * Holds if the content set `c` is callback like.
+   */
+  predicate isCallback(Lang::ContentSet c);
+
   /**
    * Gets the MaD synthetic name string representation for the content set `c`, if any.
    */
@@ -618,6 +623,20 @@ module MakeModelGenerator<
       isField(ap.getAtIndex(_))
     }
 
+    /**
+     * Holds if this access path `ap` mentions a callback.
+     */
+    private predicate mentionsCallback(PropagateContentFlow::AccessPath ap) {
+      isCallback(ap.getAtIndex(_))
+    }
+
+    /**
+     * Models as Data currently doesn't support callback logic in fields.
+     */
+    private predicate validateAccessPath(PropagateContentFlow::AccessPath ap) {
+      not (mentionsField(ap) and mentionsCallback(ap))
+    }
+
     private predicate apiFlow(
       DataFlowSummaryTargetApi api, DataFlow::ParameterNode p,
       PropagateContentFlow::AccessPath reads, ReturnNodeExt returnNodeExt,
@@ -859,6 +878,8 @@ module MakeModelGenerator<
         input = parameterNodeAsContentInput(p) + printReadAccessPath(reads) and
         output = getContentOutput(returnNodeExt) + printStoreAccessPath(stores) and
         input != output and
+        validateAccessPath(reads) and
+        validateAccessPath(stores) and
         (
           if mentionsField(reads) or mentionsField(stores)
           then lift = false and api.isRelevant()

From e9c9519d90df7ac7c458d9761cf216e2642309ed Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Wed, 6 Nov 2024 16:28:01 +0100
Subject: [PATCH 6/7] C#: Address review comments.

---
 .../dataflow/internal/DataFlowPrivate.qll     | 18 +++++++--------
 .../modelgenerator/dataflow/Summaries.cs      | 22 +++++++++----------
 .../internal/ModelGeneratorImpl.qll           |  6 ++++-
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
index ff3531e677de..5d93fb8f57ec 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -1149,7 +1149,7 @@ private module Cached {
     } or
     TCapturedVariableContent(VariableCapture::CapturedVariable v) or
     TDelegateCallArgumentContent(int i) {
-      i = [0 .. max(int j | j = any(DelegateCall dc).getNumberOfArguments())]
+      i = [0 .. max(any(DelegateCall dc).getNumberOfArguments())]
     } or
     TDelegateCallReturnContent()
 
@@ -2287,10 +2287,10 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
  * If there is a delegate call f(x), then we store "x" on "f"
  * using a delegate argument content set.
  */
-private predicate storeStepDelegateCall(Node node1, ContentSet c, Node node2) {
-  exists(DelegateCall call, int i |
-    node1.asExpr() = call.getArgument(i) and
-    node2.(PostUpdateNode).getPreUpdateNode().asExpr() = call.getExpr() and
+private predicate storeStepDelegateCall(ExplicitArgumentNode node1, ContentSet c, Node node2) {
+  exists(ExplicitDelegateLikeDataFlowCall call, int i |
+    node1.argumentOf(call, TPositionalArgumentPosition(i)) and
+    lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
     c.isDelegateCallArgument(i)
   )
 }
@@ -2456,10 +2456,10 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
  * If there is a delegate call f(x), then we read the return of the delegate
  * call.
  */
-private predicate readStepDelegateCall(Node node1, ContentSet c, Node node2) {
-  exists(DelegateCall call |
-    node1.asExpr() = call.getExpr() and
-    node2.asExpr() = call and
+private predicate readStepDelegateCall(Node node1, ContentSet c, OutNode node2) {
+  exists(ExplicitDelegateLikeDataFlowCall call |
+    lambdaCall(call, _, node1) and
+    node2.getCall(TNormalReturnKind()) = call and
     c.isDelegateCallReturn()
   )
 }
diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
index f1b8272d6b94..2d8bbc8912bc 100644
--- a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
+++ b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
@@ -64,10 +64,10 @@ public string ReturnField()
     }
 
     public Func<object, object> MyFunction;
-    // summary=Models;BasicFlow;false;MapMyFunction;(System.Object);;Argument[0];Argument[this];taint;df-generated
-    // summary=Models;BasicFlow;false;MapMyFunction;(System.Object);;Argument[this];ReturnValue;taint;df-generated
+    // summary=Models;BasicFlow;false;ApplyMyFunction;(System.Object);;Argument[0];Argument[this];taint;df-generated
+    // summary=Models;BasicFlow;false;ApplyMyFunction;(System.Object);;Argument[this];ReturnValue;taint;df-generated
     // No content based flow as MaD doesn't support callback logic in fields and properties.
-    public object MapMyFunction(object o)
+    public object ApplyMyFunction(object o)
     {
         return MyFunction(o);
     }
@@ -517,18 +517,18 @@ public string M1(string s, Func<string, string> map)
         return s;
     }
 
-    // neutral=Models;HigherOrderParameters;Map;(System.Func<System.Object,System.Object>,System.Object);summary;df-generated
-    // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
-    // contentbased-summary=Models;HigherOrderParameters;false;Map;(System.Func<System.Object,System.Object>,System.Object);;Argument[0].ReturnValue;ReturnValue;value;dfc-generated
-    public object Map(Func<object, object> f, object o)
+    // neutral=Models;HigherOrderParameters;Apply;(System.Func<System.Object,System.Object>,System.Object);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Func<System.Object,System.Object>,System.Object);;Argument[0].ReturnValue;ReturnValue;value;dfc-generated
+    public object Apply(Func<object, object> f, object o)
     {
         return f(o);
     }
 
-    // neutral=Models;HigherOrderParameters;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);summary;df-generated
-    // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[0];Argument[1].Parameter[1];value;dfc-generated
-    // contentbased-summary=Models;HigherOrderParameters;false;Map2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[1].ReturnValue;ReturnValue;value;dfc-generated
-    public object Map2(object o, Func<object, object, object> f)
+    // neutral=Models;HigherOrderParameters;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);summary;df-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[0];Argument[1].Parameter[1];value;dfc-generated
+    // contentbased-summary=Models;HigherOrderParameters;false;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[1].ReturnValue;ReturnValue;value;dfc-generated
+    public object Apply2(object o, Func<object, object, object> f)
     {
         var x = f(null, o);
         return x;
diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
index bcd1b8a19c5c..2e22ae2c2ba8 100644
--- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
+++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll
@@ -631,7 +631,11 @@ module MakeModelGenerator<
     }
 
     /**
-     * Models as Data currently doesn't support callback logic in fields.
+     * Holds if the access path `ap` is not a parameter or returnvalue of a callback
+     * stored in a field.
+     *
+     * That is, we currently don't include summaries that rely on parameters or return values
+     * of callbacks stored in fields.
      */
     private predicate validateAccessPath(PropagateContentFlow::AccessPath ap) {
       not (mentionsField(ap) and mentionsCallback(ap))

From 8041f00bf551194ae7b85635e2e930999b0d3bc6 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Thu, 7 Nov 2024 09:24:26 +0100
Subject: [PATCH 7/7] C#: Address more review comments.

---
 .../semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
index 5d93fb8f57ec..cab164846e2d 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -1149,7 +1149,7 @@ private module Cached {
     } or
     TCapturedVariableContent(VariableCapture::CapturedVariable v) or
     TDelegateCallArgumentContent(int i) {
-      i = [0 .. max(any(DelegateCall dc).getNumberOfArguments())]
+      i = [0 .. max(any(DelegateLikeCall dc).getNumberOfArguments()) - 1]
     } or
     TDelegateCallReturnContent()