From 25fda206b258685436259b82e506948c11fcd4c9 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 17 May 2022 15:17:27 +0200
Subject: [PATCH 1/8] Java: Prevent accidental recursion through
 AdditionalValueStep.

---
 .../java/dataflow/internal/DataFlowUtil.qll   | 19 ++++++++++-----
 .../dataflow/internal/FlowSummaryImpl.qll     | 23 +++++++++++++++----
 .../dataflow/internal/TaintTrackingUtil.qll   |  2 +-
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index 284d66803d90..927f17d4255e 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -100,14 +100,15 @@ predicate hasNonlocalValue(FieldRead fr) {
 /**
  * Holds if data can flow from `node1` to `node2` in one local step.
  */
+cached
 predicate localFlowStep(Node node1, Node node2) {
-  simpleLocalFlowStep(node1, node2)
+  simpleLocalFlowStep0(node1, node2)
   or
   adjacentUseUse(node1.asExpr(), node2.asExpr())
   or
   // Simple flow through library code is included in the exposed local
   // step relation, even though flow is technically inter-procedural
-  FlowSummaryImpl::Private::Steps::summaryThroughStep(node1, node2, true)
+  FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2)
 }
 
 /**
@@ -118,6 +119,16 @@ predicate localFlowStep(Node node1, Node node2) {
  */
 cached
 predicate simpleLocalFlowStep(Node node1, Node node2) {
+  simpleLocalFlowStep0(node1, node2)
+  or
+  any(AdditionalValueStep a).step(node1, node2) and
+  pragma[only_bind_out](node1.getEnclosingCallable()) =
+    pragma[only_bind_out](node2.getEnclosingCallable()) and
+  // prevent recursive call
+  (any() or strictcount(Node n1, Node n2, AdditionalValueStep a | a.step(n1, n2)) < 0)
+}
+
+private predicate simpleLocalFlowStep0(Node node1, Node node2) {
   TaintTrackingUtil::forceCachingInSameStage() and
   // Variable flow steps through adjacent def-use and use-use pairs.
   exists(SsaExplicitUpdate upd |
@@ -166,10 +177,6 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
   )
   or
   FlowSummaryImpl::Private::Steps::summaryLocalStep(node1, node2, true)
-  or
-  any(AdditionalValueStep a).step(node1, node2) and
-  pragma[only_bind_out](node1.getEnclosingCallable()) =
-    pragma[only_bind_out](node2.getEnclosingCallable())
 }
 
 private newtype TContent =
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 439d70175e20..8654df865186 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
@@ -791,15 +791,30 @@ module Private {
     }
 
     /**
-     * Holds if `arg` flows to `out` using a simple flow summary, that is, a flow
-     * summary without reads and stores.
+     * Holds if `arg` flows to `out` using a simple value-preserving flow
+     * summary, that is, a flow summary without reads and stores.
      *
      * NOTE: This step should not be used in global data-flow/taint-tracking, but may
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
-    predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
+    predicate summaryThroughStepValue(ArgNode arg, Node out) {
+      exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
+        summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
+        ret.getKind() = rk and
+        out = getAnOutNode(call, rk)
+      )
+    }
+
+    /**
+     * Holds if `arg` flows to `out` using a simple flow summary involving taint
+     * step, that is, a flow summary without reads and stores.
+     *
+     * NOTE: This step should not be used in global data-flow/taint-tracking, but may
+     * be useful to include in the exposed local data-flow/taint-tracking relations.
+     */
+    predicate summaryThroughStepTaint(ArgNode arg, Node out) {
       exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
       )
     }
 
diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 6b921a93e6ff..62269faca71b 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -51,7 +51,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::summaryThroughStep(src, sink, false)
+    FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(src, sink)
     or
     // Treat container flow as taint for the local taint flow relation
     exists(DataFlow::Content c | containerContent(c) |

From 829eb7f7a50c463bf4f4874d9ddfe9ffdea1f82a Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 17 May 2022 15:18:50 +0200
Subject: [PATCH 2/8] C#/Ruby: Sync FlowSummaryImpl.

---
 .../dataflow/internal/FlowSummaryImpl.qll     | 23 +++++++++++++++----
 .../dataflow/internal/FlowSummaryImpl.qll     | 23 +++++++++++++++----
 2 files changed, 38 insertions(+), 8 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 439d70175e20..8654df865186 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
@@ -791,15 +791,30 @@ module Private {
     }
 
     /**
-     * Holds if `arg` flows to `out` using a simple flow summary, that is, a flow
-     * summary without reads and stores.
+     * Holds if `arg` flows to `out` using a simple value-preserving flow
+     * summary, that is, a flow summary without reads and stores.
      *
      * NOTE: This step should not be used in global data-flow/taint-tracking, but may
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
-    predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
+    predicate summaryThroughStepValue(ArgNode arg, Node out) {
+      exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
+        summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
+        ret.getKind() = rk and
+        out = getAnOutNode(call, rk)
+      )
+    }
+
+    /**
+     * Holds if `arg` flows to `out` using a simple flow summary involving taint
+     * step, that is, a flow summary without reads and stores.
+     *
+     * NOTE: This step should not be used in global data-flow/taint-tracking, but may
+     * be useful to include in the exposed local data-flow/taint-tracking relations.
+     */
+    predicate summaryThroughStepTaint(ArgNode arg, Node out) {
       exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
       )
     }
 
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
index 439d70175e20..8654df865186 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -791,15 +791,30 @@ module Private {
     }
 
     /**
-     * Holds if `arg` flows to `out` using a simple flow summary, that is, a flow
-     * summary without reads and stores.
+     * Holds if `arg` flows to `out` using a simple value-preserving flow
+     * summary, that is, a flow summary without reads and stores.
      *
      * NOTE: This step should not be used in global data-flow/taint-tracking, but may
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
-    predicate summaryThroughStep(ArgNode arg, Node out, boolean preservesValue) {
+    predicate summaryThroughStepValue(ArgNode arg, Node out) {
+      exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
+        summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
+        ret.getKind() = rk and
+        out = getAnOutNode(call, rk)
+      )
+    }
+
+    /**
+     * Holds if `arg` flows to `out` using a simple flow summary involving taint
+     * step, that is, a flow summary without reads and stores.
+     *
+     * NOTE: This step should not be used in global data-flow/taint-tracking, but may
+     * be useful to include in the exposed local data-flow/taint-tracking relations.
+     */
+    predicate summaryThroughStepTaint(ArgNode arg, Node out) {
       exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, preservesValue)
+        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
       )
     }
 

From 48ab5b2403246b0e423845d0355e8501f81f7467 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 17 May 2022 15:24:59 +0200
Subject: [PATCH 3/8] C#/Ruby/Java: Fix references.

---
 .../semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll  | 2 +-
 .../code/csharp/dataflow/internal/TaintTrackingPrivate.qll    | 2 +-
 .../ql/test/library-tests/dataflow/external-models/steps.ql   | 4 +++-
 java/ql/test/library-tests/dataflow/external-models/steps.ql  | 2 +-
 .../local-additional-taint/localAdditionalTaintStep.ql        | 2 +-
 ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll | 2 +-
 .../codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll    | 2 +-
 7 files changed, 9 insertions(+), 7 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 9a5628034738..c7fdc61a3901 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -760,7 +760,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::summaryThroughStep(nodeFrom, nodeTo, true)
+    FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo)
   }
 
   cached
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
index 2df41e00299c..1dc0d22f1b2e 100755
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll
@@ -117,7 +117,7 @@ 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::summaryThroughStep(nodeFrom, nodeTo, false)
+      FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo)
       or
       // Taint collection by adding a tainted element
       exists(DataFlow::ElementContent c |
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/steps.ql b/csharp/ql/test/library-tests/dataflow/external-models/steps.ql
index ecd7ac5c6fd8..15a913272774 100644
--- a/csharp/ql/test/library-tests/dataflow/external-models/steps.ql
+++ b/csharp/ql/test/library-tests/dataflow/external-models/steps.ql
@@ -31,7 +31,9 @@ class SummaryModelTest extends SummaryModelCsv {
 query predicate summaryThroughStep(
   DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue
 ) {
-  FlowSummaryImpl::Private::Steps::summaryThroughStep(node1, node2, preservesValue)
+  FlowSummaryImpl::Private::Steps::summaryThroughStepValue(node1, node2) and preservesValue = true
+  or
+  FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2) and preservesValue = false
 }
 
 query predicate summaryGetterStep(DataFlow::Node arg, DataFlow::Node out, Content c) {
diff --git a/java/ql/test/library-tests/dataflow/external-models/steps.ql b/java/ql/test/library-tests/dataflow/external-models/steps.ql
index 09c5ca977649..ea6981742be1 100644
--- a/java/ql/test/library-tests/dataflow/external-models/steps.ql
+++ b/java/ql/test/library-tests/dataflow/external-models/steps.ql
@@ -22,5 +22,5 @@ class SummaryModelTest extends SummaryModelCsv {
 }
 
 from DataFlow::Node node1, DataFlow::Node node2
-where FlowSummaryImpl::Private::Steps::summaryThroughStep(node1, node2, false)
+where FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(node1, node2)
 select node1, node2
diff --git a/java/ql/test/library-tests/dataflow/local-additional-taint/localAdditionalTaintStep.ql b/java/ql/test/library-tests/dataflow/local-additional-taint/localAdditionalTaintStep.ql
index 977d332c36ad..66f33278a3dc 100644
--- a/java/ql/test/library-tests/dataflow/local-additional-taint/localAdditionalTaintStep.ql
+++ b/java/ql/test/library-tests/dataflow/local-additional-taint/localAdditionalTaintStep.ql
@@ -16,7 +16,7 @@ from DataFlow::Node src, DataFlow::Node sink
 where
   (
     localAdditionalTaintStep(src, sink) or
-    FlowSummaryImpl::Private::Steps::summaryThroughStep(src, sink, false)
+    FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(src, sink)
   ) and
   not FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false) and
   not FlowSummaryImpl::Private::Steps::summaryReadStep(src, _, sink) and
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
index f90112bd05ac..676b623ff880 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -270,7 +270,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::summaryThroughStep(nodeFrom, nodeTo, true)
+    FlowSummaryImpl::Private::Steps::summaryThroughStepValue(nodeFrom, nodeTo)
   }
 
   /** This is the local flow predicate that is used in type tracking. */
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll
index f9e3229188f8..94719a02ae6a 100755
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/TaintTrackingPrivate.qll
@@ -119,7 +119,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::summaryThroughStep(nodeFrom, nodeTo, false)
+    FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(nodeFrom, nodeTo)
   }
 }
 

From d4c9fddae379b2d3ecad82d9c8368783187301f1 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 17 May 2022 15:25:29 +0200
Subject: [PATCH 4/8] Java: Use fastTC.

---
 .../lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll   | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index 927f17d4255e..c21b7205bc95 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -75,7 +75,9 @@ private module ThisFlow {
  * local (intra-procedural) steps.
  */
 pragma[inline]
-predicate localFlow(Node node1, Node node2) { localFlowStep*(node1, node2) }
+predicate localFlow(Node node1, Node node2) { node1 = node2 or localFlowStepPlus(node1, node2) }
+
+private predicate localFlowStepPlus(Node node1, Node node2) = fastTC(localFlowStep/2)(node1, node2)
 
 /**
  * Holds if data can flow from `e1` to `e2` in zero or more

From a4a004a32251d6eefa50a57b09c8624b8099213e Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Tue, 17 May 2022 15:57:48 +0200
Subject: [PATCH 5/8] Java: Simplify recursion prevention.

---
 java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index c21b7205bc95..cc29cfcba849 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -127,7 +127,7 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
   pragma[only_bind_out](node1.getEnclosingCallable()) =
     pragma[only_bind_out](node2.getEnclosingCallable()) and
   // prevent recursive call
-  (any() or strictcount(Node n1, Node n2, AdditionalValueStep a | a.step(n1, n2)) < 0)
+  (any(AdditionalValueStep a).step(_, _) implies any())
 }
 
 private predicate simpleLocalFlowStep0(Node node1, Node node2) {

From af7df792890e1f6c3574735418b5cc4a3ddc990c Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Wed, 18 May 2022 09:38:11 +0200
Subject: [PATCH 6/8] Autoformat

---
 .../semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll  | 4 +---
 .../semmle/code/java/dataflow/internal/FlowSummaryImpl.qll    | 4 +---
 ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll | 4 +---
 3 files changed, 3 insertions(+), 9 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 8654df865186..d159e7cba398 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
@@ -813,9 +813,7 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStepTaint(ArgNode arg, Node out) {
-      exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
-      )
+      exists(ReturnNodeExt ret | summaryLocalStep(summaryArgParam(arg, ret, out), ret, false))
     }
 
     /**
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 8654df865186..d159e7cba398 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
@@ -813,9 +813,7 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStepTaint(ArgNode arg, Node out) {
-      exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
-      )
+      exists(ReturnNodeExt ret | summaryLocalStep(summaryArgParam(arg, ret, out), ret, false))
     }
 
     /**
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
index 8654df865186..d159e7cba398 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -813,9 +813,7 @@ module Private {
      * be useful to include in the exposed local data-flow/taint-tracking relations.
      */
     predicate summaryThroughStepTaint(ArgNode arg, Node out) {
-      exists(ReturnNodeExt ret |
-        summaryLocalStep(summaryArgParam(arg, ret, out), ret, false)
-      )
+      exists(ReturnNodeExt ret | summaryLocalStep(summaryArgParam(arg, ret, out), ret, false))
     }
 
     /**

From 0e830f6052b68599eeb9fd0a440b41317a8b6ff3 Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Thu, 19 May 2022 11:26:38 +0200
Subject: [PATCH 7/8] C#/Ruby/Java: Fix pragmas.

---
 .../code/csharp/dataflow/internal/FlowSummaryImpl.qll       | 6 +++---
 .../semmle/code/java/dataflow/internal/FlowSummaryImpl.qll  | 6 +++---
 .../lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll   | 6 +++---
 3 files changed, 9 insertions(+), 9 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 d159e7cba398..2ee1571c0d9a 100644
--- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
+++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll
@@ -785,7 +785,7 @@ module Private {
     private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
       exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        ret.getKind() = pragma[only_bind_into](rk) and
         out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
@@ -800,8 +800,8 @@ module Private {
     predicate summaryThroughStepValue(ArgNode arg, Node out) {
       exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
         summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
-        ret.getKind() = rk and
-        out = getAnOutNode(call, rk)
+        ret.getKind() = pragma[only_bind_into](rk) and
+        out = getAnOutNode(call, pragma[only_bind_into](rk))
       )
     }
 
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 d159e7cba398..2ee1571c0d9a 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll
@@ -785,7 +785,7 @@ module Private {
     private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
       exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        ret.getKind() = pragma[only_bind_into](rk) and
         out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
@@ -800,8 +800,8 @@ module Private {
     predicate summaryThroughStepValue(ArgNode arg, Node out) {
       exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
         summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
-        ret.getKind() = rk and
-        out = getAnOutNode(call, rk)
+        ret.getKind() = pragma[only_bind_into](rk) and
+        out = getAnOutNode(call, pragma[only_bind_into](rk))
       )
     }
 
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
index d159e7cba398..2ee1571c0d9a 100644
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll
@@ -785,7 +785,7 @@ module Private {
     private ParamNode summaryArgParam(ArgNode arg, ReturnNodeExt ret, OutNodeExt out) {
       exists(DataFlowCall call, ReturnKindExt rk |
         result = summaryArgParam0(call, arg) and
-        pragma[only_bind_out](ret).getKind() = pragma[only_bind_into](rk) and
+        ret.getKind() = pragma[only_bind_into](rk) and
         out = pragma[only_bind_into](rk).getAnOutNode(call)
       )
     }
@@ -800,8 +800,8 @@ module Private {
     predicate summaryThroughStepValue(ArgNode arg, Node out) {
       exists(ReturnKind rk, ReturnNode ret, DataFlowCall call |
         summaryLocalStep(summaryArgParam0(call, arg), ret, true) and
-        ret.getKind() = rk and
-        out = getAnOutNode(call, rk)
+        ret.getKind() = pragma[only_bind_into](rk) and
+        out = getAnOutNode(call, pragma[only_bind_into](rk))
       )
     }
 

From 651d9d0a44a06d28a8dc315996a200993bd8051f Mon Sep 17 00:00:00 2001
From: Anders Schack-Mulligen <aschackmull@github.com>
Date: Thu, 19 May 2022 11:39:41 +0200
Subject: [PATCH 8/8] Java: Ensure cached predicates are in the same stage.

---
 .../java/dataflow/internal/DataFlowUtil.qll   | 59 ++++++++++---------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
index cc29cfcba849..50abd9056981 100644
--- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll
@@ -99,37 +99,42 @@ predicate hasNonlocalValue(FieldRead fr) {
   )
 }
 
-/**
- * Holds if data can flow from `node1` to `node2` in one local step.
- */
 cached
-predicate localFlowStep(Node node1, Node node2) {
-  simpleLocalFlowStep0(node1, node2)
-  or
-  adjacentUseUse(node1.asExpr(), node2.asExpr())
-  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(node1, node2)
-}
+private module Cached {
+  /**
+   * Holds if data can flow from `node1` to `node2` in one local step.
+   */
+  cached
+  predicate localFlowStep(Node node1, Node node2) {
+    simpleLocalFlowStep0(node1, node2)
+    or
+    adjacentUseUse(node1.asExpr(), node2.asExpr())
+    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(node1, node2)
+  }
 
-/**
- * INTERNAL: do not use.
- *
- * This is the local flow predicate that's used as a building block in global
- * data flow. It may have less flow than the `localFlowStep` predicate.
- */
-cached
-predicate simpleLocalFlowStep(Node node1, Node node2) {
-  simpleLocalFlowStep0(node1, node2)
-  or
-  any(AdditionalValueStep a).step(node1, node2) and
-  pragma[only_bind_out](node1.getEnclosingCallable()) =
-    pragma[only_bind_out](node2.getEnclosingCallable()) and
-  // prevent recursive call
-  (any(AdditionalValueStep a).step(_, _) implies any())
+  /**
+   * INTERNAL: do not use.
+   *
+   * This is the local flow predicate that's used as a building block in global
+   * data flow. It may have less flow than the `localFlowStep` predicate.
+   */
+  cached
+  predicate simpleLocalFlowStep(Node node1, Node node2) {
+    simpleLocalFlowStep0(node1, node2)
+    or
+    any(AdditionalValueStep a).step(node1, node2) and
+    pragma[only_bind_out](node1.getEnclosingCallable()) =
+      pragma[only_bind_out](node2.getEnclosingCallable()) and
+    // prevent recursive call
+    (any(AdditionalValueStep a).step(_, _) implies any())
+  }
 }
 
+import Cached
+
 private predicate simpleLocalFlowStep0(Node node1, Node node2) {
   TaintTrackingUtil::forceCachingInSameStage() and
   // Variable flow steps through adjacent def-use and use-use pairs.