From 0c1285f5d97ae6820bfc7dc4507df0cefbae0a42 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 30 Sep 2021 13:31:31 +0200
Subject: [PATCH 1/6] Data flow: Restrict derived flow summaries

---
 .../dataflow/internal/FlowSummaryImpl.qll     | 105 ++++++++++++++----
 1 file changed, 82 insertions(+), 23 deletions(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
index 5955285bd6f0..3af1c21a517f 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
@@ -85,6 +85,9 @@ module Public {
     /** Holds if this stack contains summary component `c`. */
     predicate contains(SummaryComponent c) { c = this.drop(_).head() }
 
+    /** Gets the bottom element of this stack. */
+    SummaryComponent bottom() { result = this.drop(this.length() - 1).head() }
+
     /** Gets a textual representation of this stack. */
     string toString() {
       exists(SummaryComponent head, SummaryComponentStack tail |
@@ -197,6 +200,8 @@ module Private {
       or
       tail.(RequiredSummaryComponentStack).required(TParameterSummaryComponent(_)) and
       head = thisParam()
+      or
+      derivedFluentFlowPush(_, _, _, head, tail, _)
     }
 
   pragma[nomagic]
@@ -210,7 +215,7 @@ module Private {
     c.propagatesFlow(output, input, preservesValue) and
     preservesValue = true and
     isCallbackParameter(input) and
-    isContentOfArgument(output)
+    isContentOfArgument(output, _)
     or
     // flow from the receiver of a callback into the instance-parameter
     exists(SummaryComponentStack s, SummaryComponentStack callbackRef |
@@ -222,16 +227,81 @@ module Private {
       output = TConsSummaryComponentStack(thisParam(), input) and
       preservesValue = true
     )
+    or
+    exists(SummaryComponentStack arg, SummaryComponentStack return |
+      derivedFluentFlow(c, input, arg, return, preservesValue)
+    |
+      arg.length() = 1 and
+      output = return
+      or
+      exists(SummaryComponent head, SummaryComponentStack tail |
+        derivedFluentFlowPush(c, input, arg, head, tail, 0) and
+        output = SummaryComponentStack::push(head, tail)
+      )
+    )
+    or
+    // Chain together summaries where values get passed into callbacks along the way
+    exists(SummaryComponentStack mid, boolean preservesValue1, boolean preservesValue2 |
+      c.propagatesFlow(input, mid, preservesValue1) and
+      c.propagatesFlow(mid, output, preservesValue2) and
+      mid.drop(mid.length() - 2) =
+        SummaryComponentStack::push(TParameterSummaryComponent(_),
+          SummaryComponentStack::singleton(TArgumentSummaryComponent(_))) and
+      preservesValue = preservesValue1.booleanAnd(preservesValue2)
+    )
+  }
+
+  /**
+   * Holds if `c` has a flow summary from `input` to `arg`, where `arg`
+   * writes to (contents of) the `i`th argument, and `c` has a
+   * value-preserving flow summary from the `i`th argument to a return value
+   * (`return`).
+   *
+   * In such a case, we derive flow from `input` to (contents of) the return
+   * value.
+   *
+   * As an example, this simplifies modeling of fluent methods:
+   * for `StringBuilder.append(x)` with a specified value flow from qualifier to
+   * return value and taint flow from argument 0 to the qualifier, then this
+   * allows us to infer taint flow from argument 0 to the return value.
+   */
+  pragma[nomagic]
+  private predicate derivedFluentFlow(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponentStack return, boolean preservesValue
+  ) {
+    exists(int i |
+      summary(c, input, arg, preservesValue) and
+      isContentOfArgument(arg, i) and
+      summary(c, SummaryComponentStack::singleton(TArgumentSummaryComponent(i)), return, true) and
+      return.bottom() = TReturnSummaryComponent(_)
+    )
+  }
+
+  pragma[nomagic]
+  private predicate derivedFluentFlowPush(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponent head, SummaryComponentStack tail, int i
+  ) {
+    derivedFluentFlow(c, input, arg, tail, _) and
+    head = arg.drop(i).head() and
+    i = arg.length() - 2
+    or
+    exists(SummaryComponent head0, SummaryComponentStack tail0 |
+      derivedFluentFlowPush(c, input, arg, head0, tail0, i + 1) and
+      head = arg.drop(i).head() and
+      tail = SummaryComponentStack::push(head0, tail0)
+    )
   }
 
   private predicate isCallbackParameter(SummaryComponentStack s) {
     s.head() = TParameterSummaryComponent(_) and exists(s.tail())
   }
 
-  private predicate isContentOfArgument(SummaryComponentStack s) {
-    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail())
+  private predicate isContentOfArgument(SummaryComponentStack s, int i) {
+    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail(), i)
     or
-    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_))
+    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(i))
   }
 
   private predicate outputState(SummarizedCallable c, SummaryComponentStack s) {
@@ -508,9 +578,14 @@ module Private {
    * node, and back out to `p`.
    */
   predicate summaryAllowParameterReturnInSelf(ParamNode p) {
-    exists(SummarizedCallable c, int i |
-      c.clearsContent(i, _) and
-      p.isParameterOf(c, i)
+    exists(SummarizedCallable c, int i | p.isParameterOf(c, i) |
+      c.clearsContent(i, _)
+      or
+      exists(SummaryComponentStack inputContents, SummaryComponentStack outputContents |
+        summary(c, inputContents, outputContents, _) and
+        inputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i)) and
+        outputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i))
+      )
     )
   }
 
@@ -534,22 +609,6 @@ module Private {
         preservesValue = false and not summary(c, inputContents, outputContents, true)
       )
       or
-      // If flow through a method updates a parameter from some input A, and that
-      // parameter also is returned through B, then we'd like a combined flow from A
-      // to B as well. As an example, this simplifies modeling of fluent methods:
-      // for `StringBuilder.append(x)` with a specified value flow from qualifier to
-      // return value and taint flow from argument 0 to the qualifier, then this
-      // allows us to infer taint flow from argument 0 to the return value.
-      succ instanceof ParamNode and
-      summaryPostUpdateNode(pred, succ) and
-      preservesValue = true
-      or
-      // Similarly we would like to chain together summaries where values get passed
-      // into callbacks along the way.
-      pred instanceof ArgNode and
-      summaryPostUpdateNode(succ, pred) and
-      preservesValue = true
-      or
       exists(SummarizedCallable c, int i |
         pred.(ParamNode).isParameterOf(c, i) and
         succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and

From ac414517988fd504253842882da7d28e9bcb513d Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 21 Oct 2021 11:37:36 +0200
Subject: [PATCH 2/6] Data flow: Sync files

---
 .../dataflow/internal/FlowSummaryImpl.qll     | 105 ++++++++++++++----
 1 file changed, 82 insertions(+), 23 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
index 5955285bd6f0..3af1c21a517f 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
@@ -85,6 +85,9 @@ module Public {
     /** Holds if this stack contains summary component `c`. */
     predicate contains(SummaryComponent c) { c = this.drop(_).head() }
 
+    /** Gets the bottom element of this stack. */
+    SummaryComponent bottom() { result = this.drop(this.length() - 1).head() }
+
     /** Gets a textual representation of this stack. */
     string toString() {
       exists(SummaryComponent head, SummaryComponentStack tail |
@@ -197,6 +200,8 @@ module Private {
       or
       tail.(RequiredSummaryComponentStack).required(TParameterSummaryComponent(_)) and
       head = thisParam()
+      or
+      derivedFluentFlowPush(_, _, _, head, tail, _)
     }
 
   pragma[nomagic]
@@ -210,7 +215,7 @@ module Private {
     c.propagatesFlow(output, input, preservesValue) and
     preservesValue = true and
     isCallbackParameter(input) and
-    isContentOfArgument(output)
+    isContentOfArgument(output, _)
     or
     // flow from the receiver of a callback into the instance-parameter
     exists(SummaryComponentStack s, SummaryComponentStack callbackRef |
@@ -222,16 +227,81 @@ module Private {
       output = TConsSummaryComponentStack(thisParam(), input) and
       preservesValue = true
     )
+    or
+    exists(SummaryComponentStack arg, SummaryComponentStack return |
+      derivedFluentFlow(c, input, arg, return, preservesValue)
+    |
+      arg.length() = 1 and
+      output = return
+      or
+      exists(SummaryComponent head, SummaryComponentStack tail |
+        derivedFluentFlowPush(c, input, arg, head, tail, 0) and
+        output = SummaryComponentStack::push(head, tail)
+      )
+    )
+    or
+    // Chain together summaries where values get passed into callbacks along the way
+    exists(SummaryComponentStack mid, boolean preservesValue1, boolean preservesValue2 |
+      c.propagatesFlow(input, mid, preservesValue1) and
+      c.propagatesFlow(mid, output, preservesValue2) and
+      mid.drop(mid.length() - 2) =
+        SummaryComponentStack::push(TParameterSummaryComponent(_),
+          SummaryComponentStack::singleton(TArgumentSummaryComponent(_))) and
+      preservesValue = preservesValue1.booleanAnd(preservesValue2)
+    )
+  }
+
+  /**
+   * Holds if `c` has a flow summary from `input` to `arg`, where `arg`
+   * writes to (contents of) the `i`th argument, and `c` has a
+   * value-preserving flow summary from the `i`th argument to a return value
+   * (`return`).
+   *
+   * In such a case, we derive flow from `input` to (contents of) the return
+   * value.
+   *
+   * As an example, this simplifies modeling of fluent methods:
+   * for `StringBuilder.append(x)` with a specified value flow from qualifier to
+   * return value and taint flow from argument 0 to the qualifier, then this
+   * allows us to infer taint flow from argument 0 to the return value.
+   */
+  pragma[nomagic]
+  private predicate derivedFluentFlow(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponentStack return, boolean preservesValue
+  ) {
+    exists(int i |
+      summary(c, input, arg, preservesValue) and
+      isContentOfArgument(arg, i) and
+      summary(c, SummaryComponentStack::singleton(TArgumentSummaryComponent(i)), return, true) and
+      return.bottom() = TReturnSummaryComponent(_)
+    )
+  }
+
+  pragma[nomagic]
+  private predicate derivedFluentFlowPush(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponent head, SummaryComponentStack tail, int i
+  ) {
+    derivedFluentFlow(c, input, arg, tail, _) and
+    head = arg.drop(i).head() and
+    i = arg.length() - 2
+    or
+    exists(SummaryComponent head0, SummaryComponentStack tail0 |
+      derivedFluentFlowPush(c, input, arg, head0, tail0, i + 1) and
+      head = arg.drop(i).head() and
+      tail = SummaryComponentStack::push(head0, tail0)
+    )
   }
 
   private predicate isCallbackParameter(SummaryComponentStack s) {
     s.head() = TParameterSummaryComponent(_) and exists(s.tail())
   }
 
-  private predicate isContentOfArgument(SummaryComponentStack s) {
-    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail())
+  private predicate isContentOfArgument(SummaryComponentStack s, int i) {
+    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail(), i)
     or
-    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_))
+    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(i))
   }
 
   private predicate outputState(SummarizedCallable c, SummaryComponentStack s) {
@@ -508,9 +578,14 @@ module Private {
    * node, and back out to `p`.
    */
   predicate summaryAllowParameterReturnInSelf(ParamNode p) {
-    exists(SummarizedCallable c, int i |
-      c.clearsContent(i, _) and
-      p.isParameterOf(c, i)
+    exists(SummarizedCallable c, int i | p.isParameterOf(c, i) |
+      c.clearsContent(i, _)
+      or
+      exists(SummaryComponentStack inputContents, SummaryComponentStack outputContents |
+        summary(c, inputContents, outputContents, _) and
+        inputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i)) and
+        outputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i))
+      )
     )
   }
 
@@ -534,22 +609,6 @@ module Private {
         preservesValue = false and not summary(c, inputContents, outputContents, true)
       )
       or
-      // If flow through a method updates a parameter from some input A, and that
-      // parameter also is returned through B, then we'd like a combined flow from A
-      // to B as well. As an example, this simplifies modeling of fluent methods:
-      // for `StringBuilder.append(x)` with a specified value flow from qualifier to
-      // return value and taint flow from argument 0 to the qualifier, then this
-      // allows us to infer taint flow from argument 0 to the return value.
-      succ instanceof ParamNode and
-      summaryPostUpdateNode(pred, succ) and
-      preservesValue = true
-      or
-      // Similarly we would like to chain together summaries where values get passed
-      // into callbacks along the way.
-      pred instanceof ArgNode and
-      summaryPostUpdateNode(succ, pred) and
-      preservesValue = true
-      or
       exists(SummarizedCallable c, int i |
         pred.(ParamNode).isParameterOf(c, i) and
         succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and

From 58dd75881c6c042edc1d8a668b715b8f51ae5336 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 21 Oct 2021 14:07:07 +0200
Subject: [PATCH 3/6] C#: Update flow summary to avoid negative recursion

---
 .../csharp/frameworks/EntityFramework.qll     | 38 ++++++++++---------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll b/csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll
index 723832906c62..57bbf738841c 100644
--- a/csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll
+++ b/csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll
@@ -174,32 +174,34 @@ module EntityFramework {
     }
   }
 
-  private class RawSqlStringSummarizedCallable extends EFSummarizedCallable {
-    private SummaryComponentStack input_;
-    private SummaryComponentStack output_;
-    private boolean preservesValue_;
-
-    RawSqlStringSummarizedCallable() {
+  private class RawSqlStringConstructorSummarizedCallable extends EFSummarizedCallable {
+    RawSqlStringConstructorSummarizedCallable() {
       exists(RawSqlStringStruct s |
         this = s.getAConstructor() and
-        input_ = SummaryComponentStack::argument(0) and
-        this.getNumberOfParameters() > 0 and
-        output_ = SummaryComponentStack::return() and
-        preservesValue_ = false
-        or
-        this = s.getAConversionTo() and
-        input_ = SummaryComponentStack::argument(0) and
-        output_ = SummaryComponentStack::return() and
-        preservesValue_ = false
+        this.getNumberOfParameters() > 0
       )
     }
 
     override predicate propagatesFlow(
       SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
     ) {
-      input = input_ and
-      output = output_ and
-      preservesValue = preservesValue_
+      input = SummaryComponentStack::argument(0) and
+      output = SummaryComponentStack::return() and
+      preservesValue = false
+    }
+  }
+
+  private class RawSqlStringConversionSummarizedCallable extends EFSummarizedCallable {
+    RawSqlStringConversionSummarizedCallable() {
+      exists(RawSqlStringStruct s | this = s.getAConversionTo())
+    }
+
+    override predicate propagatesFlow(
+      SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
+    ) {
+      input = SummaryComponentStack::argument(0) and
+      output = SummaryComponentStack::return() and
+      preservesValue = false
     }
   }
 

From 3da73b900105cec750a1586af3ae6958f142cfc3 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 21 Oct 2021 11:37:49 +0200
Subject: [PATCH 4/6] C#: Update expected test output

---
 .../dataflow/external-models/ExternalFlow.cs  |  6 ++---
 .../external-models/ExternalFlow.expected     | 26 ++++++++++---------
 .../dataflow/external-models/ExternalFlow.ql  |  1 +
 .../dataflow/local/TaintTrackingStep.expected |  2 ++
 4 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
index 4125866a5493..3f223b1c69f9 100644
--- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
+++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
@@ -33,7 +33,7 @@ void M4()
 
         void M5()
         {
-            this.StepFieldSetter(new object());
+            Sink(((D)this.StepFieldSetter(new object()).Field2).Field);
             Sink(this.Field);
         }
 
@@ -102,7 +102,7 @@ void M15()
                 Sink(d);
             }, d1, d2);
             Sink(d1.Field);
-            Sink(d2.Field2); // SPURIOUS FLOW
+            Sink(d2.Field2);
         }
 
         object StepArgRes(object x) { return null; }
@@ -120,7 +120,7 @@ void StepQualArg(object @out) { }
 
         object StepFieldGetter() => throw null;
 
-        void StepFieldSetter(object value) => throw null;
+        D StepFieldSetter(object value) => throw null;
 
         object Property { get; set; }
 
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
index 107d2ecb1bfd..83478e044946 100644
--- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
+++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
@@ -11,8 +11,12 @@ edges
 | ExternalFlow.cs:30:13:30:16 | [post] this access [field Field] : Object | ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object |
 | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | ExternalFlow.cs:30:13:30:16 | [post] this access [field Field] : Object |
 | ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter |
-| ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object |
-| ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object |
+| ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object | ExternalFlow.cs:36:18:36:69 | access to field Field |
+| ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object |
+| ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object | ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object |
+| ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object | ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object |
+| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object |
+| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object |
 | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object | ExternalFlow.cs:37:18:37:27 | access to field Field |
 | ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object | ExternalFlow.cs:43:18:43:21 | this access [property Property] : Object |
 | ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object |
@@ -48,10 +52,7 @@ edges
 | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:98:13:98:14 | [post] access to local variable d1 [field Field] : Object |
 | ExternalFlow.cs:100:20:100:20 | d : Object | ExternalFlow.cs:102:22:102:22 | access to parameter d |
 | ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:100:20:100:20 | d : Object |
-| ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object |
-| ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object | ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object |
 | ExternalFlow.cs:104:18:104:19 | access to local variable d1 [field Field] : Object | ExternalFlow.cs:104:18:104:25 | access to field Field |
-| ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object | ExternalFlow.cs:105:18:105:26 | access to field Field2 |
 nodes
 | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
 | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
@@ -69,8 +70,12 @@ nodes
 | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
 | ExternalFlow.cs:31:18:31:21 | this access [field Field] : Object | semmle.label | this access [field Field] : Object |
 | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | semmle.label | call to method StepFieldGetter |
-| ExternalFlow.cs:36:13:36:16 | [post] this access [field Field] : Object | semmle.label | [post] this access [field Field] : Object |
-| ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
+| ExternalFlow.cs:36:18:36:69 | access to field Field | semmle.label | access to field Field |
+| ExternalFlow.cs:36:19:36:62 | (...) ... [field Field] : Object | semmle.label | (...) ... [field Field] : Object |
+| ExternalFlow.cs:36:22:36:25 | [post] this access [field Field] : Object | semmle.label | [post] this access [field Field] : Object |
+| ExternalFlow.cs:36:22:36:55 | call to method StepFieldSetter [field Field2, field Field] : Object | semmle.label | call to method StepFieldSetter [field Field2, field Field] : Object |
+| ExternalFlow.cs:36:22:36:62 | access to field Field2 [field Field] : Object | semmle.label | access to field Field2 [field Field] : Object |
+| ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
 | ExternalFlow.cs:37:18:37:21 | this access [field Field] : Object | semmle.label | this access [field Field] : Object |
 | ExternalFlow.cs:37:18:37:27 | access to field Field | semmle.label | access to field Field |
 | ExternalFlow.cs:42:13:42:16 | [post] this access [property Property] : Object | semmle.label | [post] this access [property Property] : Object |
@@ -116,11 +121,8 @@ nodes
 | ExternalFlow.cs:100:20:100:20 | d : Object | semmle.label | d : Object |
 | ExternalFlow.cs:102:22:102:22 | access to parameter d | semmle.label | access to parameter d |
 | ExternalFlow.cs:103:16:103:17 | access to local variable d1 [field Field] : Object | semmle.label | access to local variable d1 [field Field] : Object |
-| ExternalFlow.cs:103:20:103:21 | [post] access to local variable d2 [field Field2] : Object | semmle.label | [post] access to local variable d2 [field Field2] : Object |
 | ExternalFlow.cs:104:18:104:19 | access to local variable d1 [field Field] : Object | semmle.label | access to local variable d1 [field Field] : Object |
 | ExternalFlow.cs:104:18:104:25 | access to field Field | semmle.label | access to field Field |
-| ExternalFlow.cs:105:18:105:19 | access to local variable d2 [field Field2] : Object | semmle.label | access to local variable d2 [field Field2] : Object |
-| ExternalFlow.cs:105:18:105:26 | access to field Field2 | semmle.label | access to field Field2 |
 subpaths
 invalidModelRow
 #select
@@ -129,7 +131,8 @@ invalidModelRow
 | ExternalFlow.cs:18:18:18:24 | access to local variable argOut1 | ExternalFlow.cs:16:30:16:41 | object creation of type Object : Object | ExternalFlow.cs:18:18:18:24 | access to local variable argOut1 | $@ | ExternalFlow.cs:16:30:16:41 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:25:18:25:21 | this access | ExternalFlow.cs:23:27:23:38 | object creation of type Object : Object | ExternalFlow.cs:25:18:25:21 | this access | $@ | ExternalFlow.cs:23:27:23:38 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | ExternalFlow.cs:31:18:31:39 | call to method StepFieldGetter | $@ | ExternalFlow.cs:30:26:30:37 | object creation of type Object : Object | object creation of type Object : Object |
-| ExternalFlow.cs:37:18:37:27 | access to field Field | ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | ExternalFlow.cs:37:18:37:27 | access to field Field | $@ | ExternalFlow.cs:36:34:36:45 | object creation of type Object : Object | object creation of type Object : Object |
+| ExternalFlow.cs:36:18:36:69 | access to field Field | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:36:18:36:69 | access to field Field | $@ | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | object creation of type Object : Object |
+| ExternalFlow.cs:37:18:37:27 | access to field Field | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | ExternalFlow.cs:37:18:37:27 | access to field Field | $@ | ExternalFlow.cs:36:43:36:54 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:43:18:43:42 | call to method StepPropertyGetter | ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | ExternalFlow.cs:43:18:43:42 | call to method StepPropertyGetter | $@ | ExternalFlow.cs:42:29:42:40 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:49:18:49:30 | access to property Property | ExternalFlow.cs:48:37:48:48 | object creation of type Object : Object | ExternalFlow.cs:49:18:49:30 | access to property Property | $@ | ExternalFlow.cs:48:37:48:48 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter | ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | ExternalFlow.cs:55:18:55:41 | call to method StepElementGetter | $@ | ExternalFlow.cs:54:36:54:47 | object creation of type Object : Object | object creation of type Object : Object |
@@ -141,4 +144,3 @@ invalidModelRow
 | ExternalFlow.cs:92:18:92:18 | (...) ... | ExternalFlow.cs:90:21:90:34 | object creation of type String : String | ExternalFlow.cs:92:18:92:18 | (...) ... | $@ | ExternalFlow.cs:90:21:90:34 | object creation of type String : String | object creation of type String : String |
 | ExternalFlow.cs:102:22:102:22 | access to parameter d | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:102:22:102:22 | access to parameter d | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
 | ExternalFlow.cs:104:18:104:25 | access to field Field | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:104:18:104:25 | access to field Field | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
-| ExternalFlow.cs:105:18:105:26 | access to field Field2 | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | ExternalFlow.cs:105:18:105:26 | access to field Field2 | $@ | ExternalFlow.cs:98:24:98:35 | object creation of type Object : Object | object creation of type Object : Object |
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql
index f164967c7707..5498d8c42189 100644
--- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql
+++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql
@@ -17,6 +17,7 @@ class SummaryModelTest extends SummaryModelCsv {
         "My.Qltest;D;false;StepArgQual;(System.Object);;Argument[0];Argument[-1];taint",
         "My.Qltest;D;false;StepFieldGetter;();;Field[My.Qltest.D.Field] of Argument[-1];ReturnValue;value",
         "My.Qltest;D;false;StepFieldSetter;(System.Object);;Argument[0];Field[My.Qltest.D.Field] of Argument[-1];value",
+        "My.Qltest;D;false;StepFieldSetter;(System.Object);;Argument[-1];Field[My.Qltest.D.Field2] of ReturnValue;value",
         "My.Qltest;D;false;StepPropertyGetter;();;Property[My.Qltest.D.Property] of Argument[-1];ReturnValue;value",
         "My.Qltest;D;false;StepPropertySetter;(System.Object);;Argument[0];Property[My.Qltest.D.Property] of Argument[-1];value",
         "My.Qltest;D;false;StepElementGetter;();;Element of Argument[-1];ReturnValue;value",
diff --git a/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected b/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected
index b5a6e1ba9297..84c3f122ab74 100644
--- a/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected
+++ b/csharp/ql/test/library-tests/dataflow/local/TaintTrackingStep.expected
@@ -403,6 +403,7 @@
 | LocalDataFlow.cs:235:9:235:14 | access to local variable sink36 | LocalDataFlow.cs:235:9:235:33 | call to method AppendLine |
 | LocalDataFlow.cs:235:9:235:14 | access to local variable sink36 | LocalDataFlow.cs:236:15:236:20 | access to local variable sink36 |
 | LocalDataFlow.cs:235:27:235:32 | access to local variable sink35 | LocalDataFlow.cs:235:9:235:14 | [post] access to local variable sink36 |
+| LocalDataFlow.cs:235:27:235:32 | access to local variable sink35 | LocalDataFlow.cs:235:9:235:33 | call to method AppendLine |
 | LocalDataFlow.cs:239:13:239:51 | SSA def(nonSink10) | LocalDataFlow.cs:240:15:240:23 | access to local variable nonSink10 |
 | LocalDataFlow.cs:239:25:239:51 | object creation of type StringBuilder | LocalDataFlow.cs:239:13:239:51 | SSA def(nonSink10) |
 | LocalDataFlow.cs:239:43:239:50 | access to local variable nonSink0 | LocalDataFlow.cs:239:25:239:51 | object creation of type StringBuilder |
@@ -419,6 +420,7 @@
 | LocalDataFlow.cs:243:9:243:17 | access to local variable nonSink10 | LocalDataFlow.cs:243:9:243:38 | call to method AppendLine |
 | LocalDataFlow.cs:243:9:243:17 | access to local variable nonSink10 | LocalDataFlow.cs:244:15:244:23 | access to local variable nonSink10 |
 | LocalDataFlow.cs:243:30:243:37 | access to local variable nonSink0 | LocalDataFlow.cs:243:9:243:17 | [post] access to local variable nonSink10 |
+| LocalDataFlow.cs:243:30:243:37 | access to local variable nonSink0 | LocalDataFlow.cs:243:9:243:38 | call to method AppendLine |
 | LocalDataFlow.cs:247:13:247:52 | SSA def(taintedDataContract) | LocalDataFlow.cs:248:22:248:40 | access to local variable taintedDataContract |
 | LocalDataFlow.cs:247:13:247:52 | SSA qualifier def(taintedDataContract.AList) | LocalDataFlow.cs:250:22:250:46 | access to property AList |
 | LocalDataFlow.cs:247:13:247:52 | SSA qualifier def(taintedDataContract.AString) | LocalDataFlow.cs:248:22:248:48 | access to property AString |

From 6d58dd2823ab03ea516c8734fdc0c75dcfe960e9 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Thu, 21 Oct 2021 11:38:52 +0200
Subject: [PATCH 5/6] Java: Update expected test output

---
 .../library-tests/dataflow/callback-dispatch/A.java    | 10 +---------
 java/ql/test/library-tests/frameworks/stream/Test.java |  2 +-
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/java/ql/test/library-tests/dataflow/callback-dispatch/A.java b/java/ql/test/library-tests/dataflow/callback-dispatch/A.java
index 680badfe604c..087a93b0c4e1 100644
--- a/java/ql/test/library-tests/dataflow/callback-dispatch/A.java
+++ b/java/ql/test/library-tests/dataflow/callback-dispatch/A.java
@@ -151,15 +151,7 @@ void foo2() {
 
     forEach(new Object[] {source(16)}, x -> sink(x)); // $ flow=16
 
-    // Spurious flow from 17 is reasonable as it would likely
-    // also occur if the lambda body was inlined in a for loop.
-    // It occurs from the combination of being able to observe
-    // the side-effect of the callback on the other argument and
-    // being able to chain summaries that update/read arguments,
-    // e.g. fluent apis.
-    // Spurious flow from 18 is due to not matching call targets
-    // in a return-from-call-to-enter-call flow sequence.
-    forEach(new Object[2][], xs -> { sink(xs[0]); xs[0] = source(17); }); // $ SPURIOUS: flow=17 flow=18
+    forEach(new Object[2][], xs -> { sink(xs[0]); xs[0] = source(17); });
 
     Object[][] xss = new Object[][] { { null } };
     forEach(xss, x -> {x[0] = source(18);});
diff --git a/java/ql/test/library-tests/frameworks/stream/Test.java b/java/ql/test/library-tests/frameworks/stream/Test.java
index 040fce0158e8..331ba7a7e506 100644
--- a/java/ql/test/library-tests/frameworks/stream/Test.java
+++ b/java/ql/test/library-tests/frameworks/stream/Test.java
@@ -436,7 +436,7 @@ public void test() throws Exception {
                     sink(y); // $ hasValueFlow=reduce_3 hasValueFlow=reduce_4 hasValueFlow=reduce_5
                     return source("reduce_5");
                 });
-            sink(out); // $ hasValueFlow=reduce_4 hasValueFlow=reduce_5 SPURIOUS: hasValueFlow=reduce_3
+            sink(out); // $ hasValueFlow=reduce_4 hasValueFlow=reduce_5
         }
         {
             // "java.util.stream;Stream;true;reduce;(Object,BiFunction,BinaryOperator);;Argument[0];ReturnValue;value"

From 4eacbd1cbe820505dafec07806515995dd3963d7 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Tue, 2 Nov 2021 10:22:01 +0100
Subject: [PATCH 6/6] Ruby: Sync files

---
 .../dataflow/internal/FlowSummaryImpl.qll     | 105 ++++++++++++++----
 1 file changed, 82 insertions(+), 23 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
index 5955285bd6f0..3af1c21a517f 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -85,6 +85,9 @@ module Public {
     /** Holds if this stack contains summary component `c`. */
     predicate contains(SummaryComponent c) { c = this.drop(_).head() }
 
+    /** Gets the bottom element of this stack. */
+    SummaryComponent bottom() { result = this.drop(this.length() - 1).head() }
+
     /** Gets a textual representation of this stack. */
     string toString() {
       exists(SummaryComponent head, SummaryComponentStack tail |
@@ -197,6 +200,8 @@ module Private {
       or
       tail.(RequiredSummaryComponentStack).required(TParameterSummaryComponent(_)) and
       head = thisParam()
+      or
+      derivedFluentFlowPush(_, _, _, head, tail, _)
     }
 
   pragma[nomagic]
@@ -210,7 +215,7 @@ module Private {
     c.propagatesFlow(output, input, preservesValue) and
     preservesValue = true and
     isCallbackParameter(input) and
-    isContentOfArgument(output)
+    isContentOfArgument(output, _)
     or
     // flow from the receiver of a callback into the instance-parameter
     exists(SummaryComponentStack s, SummaryComponentStack callbackRef |
@@ -222,16 +227,81 @@ module Private {
       output = TConsSummaryComponentStack(thisParam(), input) and
       preservesValue = true
     )
+    or
+    exists(SummaryComponentStack arg, SummaryComponentStack return |
+      derivedFluentFlow(c, input, arg, return, preservesValue)
+    |
+      arg.length() = 1 and
+      output = return
+      or
+      exists(SummaryComponent head, SummaryComponentStack tail |
+        derivedFluentFlowPush(c, input, arg, head, tail, 0) and
+        output = SummaryComponentStack::push(head, tail)
+      )
+    )
+    or
+    // Chain together summaries where values get passed into callbacks along the way
+    exists(SummaryComponentStack mid, boolean preservesValue1, boolean preservesValue2 |
+      c.propagatesFlow(input, mid, preservesValue1) and
+      c.propagatesFlow(mid, output, preservesValue2) and
+      mid.drop(mid.length() - 2) =
+        SummaryComponentStack::push(TParameterSummaryComponent(_),
+          SummaryComponentStack::singleton(TArgumentSummaryComponent(_))) and
+      preservesValue = preservesValue1.booleanAnd(preservesValue2)
+    )
+  }
+
+  /**
+   * Holds if `c` has a flow summary from `input` to `arg`, where `arg`
+   * writes to (contents of) the `i`th argument, and `c` has a
+   * value-preserving flow summary from the `i`th argument to a return value
+   * (`return`).
+   *
+   * In such a case, we derive flow from `input` to (contents of) the return
+   * value.
+   *
+   * As an example, this simplifies modeling of fluent methods:
+   * for `StringBuilder.append(x)` with a specified value flow from qualifier to
+   * return value and taint flow from argument 0 to the qualifier, then this
+   * allows us to infer taint flow from argument 0 to the return value.
+   */
+  pragma[nomagic]
+  private predicate derivedFluentFlow(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponentStack return, boolean preservesValue
+  ) {
+    exists(int i |
+      summary(c, input, arg, preservesValue) and
+      isContentOfArgument(arg, i) and
+      summary(c, SummaryComponentStack::singleton(TArgumentSummaryComponent(i)), return, true) and
+      return.bottom() = TReturnSummaryComponent(_)
+    )
+  }
+
+  pragma[nomagic]
+  private predicate derivedFluentFlowPush(
+    SummarizedCallable c, SummaryComponentStack input, SummaryComponentStack arg,
+    SummaryComponent head, SummaryComponentStack tail, int i
+  ) {
+    derivedFluentFlow(c, input, arg, tail, _) and
+    head = arg.drop(i).head() and
+    i = arg.length() - 2
+    or
+    exists(SummaryComponent head0, SummaryComponentStack tail0 |
+      derivedFluentFlowPush(c, input, arg, head0, tail0, i + 1) and
+      head = arg.drop(i).head() and
+      tail = SummaryComponentStack::push(head0, tail0)
+    )
   }
 
   private predicate isCallbackParameter(SummaryComponentStack s) {
     s.head() = TParameterSummaryComponent(_) and exists(s.tail())
   }
 
-  private predicate isContentOfArgument(SummaryComponentStack s) {
-    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail())
+  private predicate isContentOfArgument(SummaryComponentStack s, int i) {
+    s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail(), i)
     or
-    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_))
+    s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(i))
   }
 
   private predicate outputState(SummarizedCallable c, SummaryComponentStack s) {
@@ -508,9 +578,14 @@ module Private {
    * node, and back out to `p`.
    */
   predicate summaryAllowParameterReturnInSelf(ParamNode p) {
-    exists(SummarizedCallable c, int i |
-      c.clearsContent(i, _) and
-      p.isParameterOf(c, i)
+    exists(SummarizedCallable c, int i | p.isParameterOf(c, i) |
+      c.clearsContent(i, _)
+      or
+      exists(SummaryComponentStack inputContents, SummaryComponentStack outputContents |
+        summary(c, inputContents, outputContents, _) and
+        inputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i)) and
+        outputContents.bottom() = pragma[only_bind_into](TArgumentSummaryComponent(i))
+      )
     )
   }
 
@@ -534,22 +609,6 @@ module Private {
         preservesValue = false and not summary(c, inputContents, outputContents, true)
       )
       or
-      // If flow through a method updates a parameter from some input A, and that
-      // parameter also is returned through B, then we'd like a combined flow from A
-      // to B as well. As an example, this simplifies modeling of fluent methods:
-      // for `StringBuilder.append(x)` with a specified value flow from qualifier to
-      // return value and taint flow from argument 0 to the qualifier, then this
-      // allows us to infer taint flow from argument 0 to the return value.
-      succ instanceof ParamNode and
-      summaryPostUpdateNode(pred, succ) and
-      preservesValue = true
-      or
-      // Similarly we would like to chain together summaries where values get passed
-      // into callbacks along the way.
-      pred instanceof ArgNode and
-      summaryPostUpdateNode(succ, pred) and
-      preservesValue = true
-      or
       exists(SummarizedCallable c, int i |
         pred.(ParamNode).isParameterOf(c, i) and
         succ = summaryNode(c, TSummaryNodeClearsContentState(i, _)) and