From 6d346dbedd27522beb0ca44829008d7717a3a114 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Wed, 28 Aug 2024 14:40:04 +0200
Subject: [PATCH 1/2] DataFlow: Bugfix in flow state for value preservation.

---
 .../dataflow/internal/ContentDataFlowImpl.qll | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
index c63f36bdeda..f4b4b9655e4 100644
--- a/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
@@ -51,6 +51,11 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
      */
     default predicate isAdditionalFlowStep(Node node1, Node node2) { none() }
 
+    /**
+     * Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow steps.
+     */
+    default predicate isAdditionalTaintStep(Node node1, Node node2) { none() }
+
     /** Holds if data flow into `node` is prohibited. */
     default predicate isBarrier(Node node) { none() }
 
@@ -101,7 +106,7 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
       predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) {
         storeStep(node1, state1, _, node2, state2) or
         readStep(node1, state1, _, node2, state2) or
-        additionalStep(node1, state1, node2, state2)
+        additionalTaintStep(node1, state1, node2, state2)
       }
 
       predicate isAdditionalFlowStep = ContentConfig::isAdditionalFlowStep/2;
@@ -229,8 +234,8 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
       )
     }
 
-    private predicate additionalStep(Node node1, State state1, Node node2, State state2) {
-      ContentConfig::isAdditionalFlowStep(node1, node2) and
+    private predicate additionalTaintStep(Node node1, State state1, Node node2, State state2) {
+      ContentConfig::isAdditionalTaintStep(node1, node2) and
       (
         state1 instanceof InitState and
         state2.(InitState).decode(false)
@@ -302,12 +307,16 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
           // relation, when flow can reach a sink without going back out
           Flow::PathGraph::subpaths(pred, succ, _, _) and
           not reachesSink(succ)
-          or
+        )
+        or
+        exists(Node predNode, State predState, Node succNode, State succState |
+          succNodeAndState(pred, predNode, predState, succ, succNode, succState)
+        |
           // needed to record store steps
-          storeStep(pred.getNode(), pred.getState(), _, succ.getNode(), succ.getState())
+          storeStep(predNode, predState, _, succNode, succState)
           or
           // needed to record read steps
-          readStep(pred.getNode(), pred.getState(), _, succ.getNode(), succ.getState())
+          readStep(predNode, predState, _, succNode, succState)
         )
       }
 

From ff31aa540c45aef55cbbe3f0815f522aaa4f1127 Mon Sep 17 00:00:00 2001
From: Michael Nebel <michaelnebel@github.com>
Date: Thu, 29 Aug 2024 15:54:04 +0200
Subject: [PATCH 2/2] Address review comments.

---
 .../dataflow/internal/ContentDataFlowImpl.qll       | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
index f4b4b9655e4..f413c6dd7ad 100644
--- a/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/ContentDataFlowImpl.qll
@@ -51,11 +51,6 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
      */
     default predicate isAdditionalFlowStep(Node node1, Node node2) { none() }
 
-    /**
-     * Holds if taint may propagate from `node1` to `node2` in addition to the normal data-flow steps.
-     */
-    default predicate isAdditionalTaintStep(Node node1, Node node2) { none() }
-
     /** Holds if data flow into `node` is prohibited. */
     default predicate isBarrier(Node node) { none() }
 
@@ -106,11 +101,9 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
       predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) {
         storeStep(node1, state1, _, node2, state2) or
         readStep(node1, state1, _, node2, state2) or
-        additionalTaintStep(node1, state1, node2, state2)
+        additionalStep(node1, state1, node2, state2)
       }
 
-      predicate isAdditionalFlowStep = ContentConfig::isAdditionalFlowStep/2;
-
       predicate isBarrier = ContentConfig::isBarrier/1;
 
       FlowFeature getAFeature() { result = ContentConfig::getAFeature() }
@@ -234,8 +227,8 @@ module MakeImplContentDataFlow<LocationSig Location, InputSig<Location> Lang> {
       )
     }
 
-    private predicate additionalTaintStep(Node node1, State state1, Node node2, State state2) {
-      ContentConfig::isAdditionalTaintStep(node1, node2) and
+    private predicate additionalStep(Node node1, State state1, Node node2, State state2) {
+      ContentConfig::isAdditionalFlowStep(node1, node2) and
       (
         state1 instanceof InitState and
         state2.(InitState).decode(false)