Merge pull request #18633 from aschackmull/dataflow/refactor-flowstate

Dataflow: Refactor FlowState to be paired with Node
github · Feb 5, 2025 · bcec7ee · bcec7ee
2 parents 670ecb3 + 73d7250
commit bcec7ee
Show file tree

Hide file tree

Showing 6 changed files with 5,241 additions and 4,901 deletions.
diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql
@@ -251,25 +251,19 @@ module PropNameTrackingConfig implements DataFlow::StateConfigSig {
     node = DataFlow::MakeStateBarrierGuard<FlowState, BarrierGuard>::getABarrierNode(state)
   }
 
-  predicate isAdditionalFlowStep(
-    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
-  ) {
-    exists(state1) and
-    state2 = state1 and
-    (
-      // Step through `p -> x[p]`
-      exists(DataFlow::PropRead read |
-        node1 = read.getPropertyNameExpr().flow() and
-        not read.(DynamicPropRead).hasDominatingAssignment() and
-        node2 = read
-      )
-      or
-      // Step through `x -> x[p]`
-      exists(DynamicPropRead read |
-        not read.hasDominatingAssignment() and
-        node1 = read.getBase() and
-        node2 = read
-      )
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    // Step through `p -> x[p]`
+    exists(DataFlow::PropRead read |
+      node1 = read.getPropertyNameExpr().flow() and
+      not read.(DynamicPropRead).hasDominatingAssignment() and
+      node2 = read
+    )
+    or
+    // Step through `x -> x[p]`
+    exists(DynamicPropRead read |
+      not read.hasDominatingAssignment() and
+      node1 = read.getBase() and
+      node2 = read
     )
   }
 

diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll
@@ -643,6 +643,7 @@ private module PathGraphSigMod {
 module DataFlowMake<LocationSig Location, InputSig<Location> Lang> {
   private import Lang
   private import internal.DataFlowImpl::MakeImpl<Location, Lang>
+  private import internal.DataFlowImplStage1::MakeImplStage1<Location, Lang>
   import Configs<Location, Lang>
 
   /**
@@ -700,7 +701,13 @@ module DataFlowMake<LocationSig Location, InputSig<Location> Lang> {
       }
     }
 
-    import Impl<C>
+    private module Stage1 = ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = Impl<C, Stage1::Stage1NoState>;
+
+    import Flow
   }
 
   /**
@@ -723,7 +730,13 @@ module DataFlowMake<LocationSig Location, InputSig<Location> Lang> {
       }
     }
 
-    import Impl<C>
+    private module Stage1 = ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = Impl<C, Stage1::Stage1WithState>;
+
+    import Flow
   }
 
   signature class PathNodeSig {

diff --git a/shared/dataflow/codeql/dataflow/TaintTracking.qll b/shared/dataflow/codeql/dataflow/TaintTracking.qll
@@ -5,6 +5,7 @@
 
 private import DataFlow as DF
 private import internal.DataFlowImpl
+private import internal.DataFlowImplStage1
 private import codeql.util.Location
 
 /**
@@ -47,6 +48,7 @@ module TaintFlowMake<
   private import TaintTrackingLang
   private import DF::DataFlowMake<Location, DataFlowLang> as DataFlow
   private import MakeImpl<Location, DataFlowLang> as DataFlowInternal
+  private import MakeImplStage1<Location, DataFlowLang> as DataFlowInternalStage1
 
   private module AddTaintDefaults<DataFlowInternal::FullStateConfigSig Config> implements
     DataFlowInternal::FullStateConfigSig
@@ -94,7 +96,13 @@ module TaintFlowMake<
       import AddTaintDefaults<Config0>
     }
 
-    import DataFlowInternal::Impl<C>
+    private module Stage1 = DataFlowInternalStage1::ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = DataFlowInternal::Impl<C, Stage1::Stage1NoState>;
+
+    import Flow
   }
 
   /**
@@ -122,7 +130,13 @@ module TaintFlowMake<
       import AddTaintDefaults<Config0>
     }
 
-    import DataFlowInternal::Impl<C>
+    private module Stage1 = DataFlowInternalStage1::ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = DataFlowInternal::Impl<C, Stage1::Stage1WithState>;
+
+    import Flow
   }
 
   signature int speculationLimitSig();
@@ -218,7 +232,13 @@ module TaintFlowMake<
       import AddTaintDefaults<AddSpeculativeTaintSteps<Config0, speculationLimit/0>>
     }
 
-    import DataFlowInternal::Impl<C>
+    private module Stage1 = DataFlowInternalStage1::ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = DataFlowInternal::Impl<C, Stage1::Stage1WithState>;
+
+    import Flow
   }
 
   /**
@@ -250,6 +270,12 @@ module TaintFlowMake<
       import AddTaintDefaults<AddSpeculativeTaintSteps<Config0, speculationLimit/0>>
     }
 
-    import DataFlowInternal::Impl<C>
+    private module Stage1 = DataFlowInternalStage1::ImplStage1<C>;
+
+    import Stage1::PartialFlow
+
+    private module Flow = DataFlowInternal::Impl<C, Stage1::Stage1WithState>;
+
+    import Flow
   }
 }