wip

Author: hvitved

shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll

@@ -109,7 +109,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     }
 
     predicate simpleLocalSmallStep(Node node1, Node node2) {
-      simpleLocalFlowStepExt(node1, node2, _)
+      simpleLocalFlowStepCached(node1, node2, _)
     }
 
     predicate levelStepNoCall(Node n1, LocalSourceNode n2) { none() }
@@ -878,7 +878,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
 
     Node asNode() { this = TNodeNormal(result) }
 
-    Node asNodeReverse(boolean b) { this = TNodeReverse(result, b) }
+    Node asNodeReverse(boolean allowFwdFlowOut) { this = TNodeReverse(result, allowFwdFlowOut) }
 
     /** Gets the corresponding Node if this is a normal node or its post-implicit read node. */
     Node asNodeOrImplicitRead() { this = TNodeNormal(result) or this = TNodeImplicitRead(result) }
@@ -926,18 +926,18 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
   }
 
   final class DataFlowCallEx extends MkDataFlowCallEx {
-    DataFlowCall asDataFlowCall(boolean b) { this = MkDataFlowCallEx(result, b) }
+    DataFlowCall asDataFlowCall(boolean allowFwdFlowOut) {
+      this = MkDataFlowCallEx(result, allowFwdFlowOut)
+    }
 
-    // DataFlowCall asDataFlowCallReverse(boolean b) { this = TReverseDataFlowCall(result, b) }
-    // DataFlowCall projectToCall() { result = [this.asDataFlowCall(), this.asDataFlowCallReverse(_)] }
     DataFlowCallable getEnclosingCallable() {
       result = this.asDataFlowCall(_).getEnclosingCallable()
     }
 
     string toString() {
-      exists(boolean b, string s |
-        s = this.asDataFlowCall(b).toString() and
-        if b = true then result = s else result = s + " (no flow out to post)"
+      exists(boolean allowFwdFlowOut, string s |
+        s = this.asDataFlowCall(allowFwdFlowOut).toString() and
+        if allowFwdFlowOut = true then result = s else result = s + " (no fwd flow out)"
       )
     }
 
@@ -975,9 +975,10 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     ArgNodeEx() {
       this.asNode().(ArgNode).argumentOf(call_.asDataFlowCall(true), pos_.asArgumentPosition())
       or
-      exists(boolean b |
-        pragma[only_bind_into](this.asNodeReverse(b)) =
-          getAnOutNode(call_.asDataFlowCall(b), pos_.asReturnKind().(ValueReturnKind).getKind())
+      exists(boolean allowFwdFlowOut |
+        pragma[only_bind_into](this.asNodeReverse(allowFwdFlowOut)) =
+          getAnOutNode(call_.asDataFlowCall(allowFwdFlowOut),
+            pos_.asReturnKind().(ValueReturnKind).getKind())
       )
     }
 
@@ -1054,9 +1055,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate forceCachingInSameStage() { any() }
 
     cached
-    newtype TDataFlowCallEx =
-      // NormalDataFlowCall(DataFlowCall call) or
-      MkDataFlowCallEx(DataFlowCall call, Boolean b)
+    newtype TDataFlowCallEx = MkDataFlowCallEx(DataFlowCall call, Boolean allowFwdFlowOut)
 
     cached
     newtype TArgumentPositionEx =
@@ -1148,16 +1147,16 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
 
     cached
     OutNodeEx getAnOutNodeEx(DataFlowCallEx call, ReturnKindExt k) {
-      exists(DataFlowCall c, boolean b | c = call.asDataFlowCall(b) |
+      exists(DataFlowCall c, boolean allowFwdFlowOut | c = call.asDataFlowCall(allowFwdFlowOut) |
         result.asNode() = getAnOutNode(c, k.(ValueReturnKind).getKind())
         or
         exists(ArgNode arg |
           arg.argumentOf(c, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition())
         |
-          b = false and
+          allowFwdFlowOut = false and
           result.asNodeReverse(_) = arg
           or
-          b = true and
+          allowFwdFlowOut = true and
           result.asNode().(PostUpdateNode).getPreUpdateNode() = arg
         )
       )
@@ -1778,9 +1777,9 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     predicate readEx(NodeEx node1, ContentSet c, NodeEx node2) {
       readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()))
       or
-      exists(boolean b |
-        storeSet(pragma[only_bind_into](node2.asNodeReverse(b)), c,
-          pragma[only_bind_into](node1.asNodeReverse(b)), _, _)
+      exists(boolean allowFwdFlowOut |
+        storeSet(pragma[only_bind_into](node2.asNodeReverse(allowFwdFlowOut)), c,
+          pragma[only_bind_into](node1.asNodeReverse(allowFwdFlowOut)), _, _)
       )
     }
 
@@ -1819,49 +1818,25 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
         storeSet(pragma[only_bind_into](node1.asNode()), cs, pragma[only_bind_into](node2.asNode()),
           contentType, containerType)
         or
-        exists(Node n1, Node n2, boolean b |
-          n1 = pragma[only_bind_into](node1.asNodeReverse(b)) and
-          n2 = pragma[only_bind_into](node2.asNodeReverse(b)) and
+        exists(Node n1, Node n2, boolean allowFwdFlowOut |
+          n1 = pragma[only_bind_into](node1.asNodeReverse(allowFwdFlowOut)) and
+          n2 = pragma[only_bind_into](node2.asNodeReverse(allowFwdFlowOut)) and
           readSet(n2, cs, n1) and
-          contentType = getNodeDataFlowType(node1.asNodeReverse(b)) and
-          containerType = getNodeDataFlowType(node2.asNodeReverse(b)) and
-          not exists(PostUpdateNode pn1, PostUpdateNode pn2 |
+          contentType = getNodeDataFlowType(n1) and
+          containerType = getNodeDataFlowType(n2) and
+          not exists(
+            PostUpdateNode pn1, PostUpdateNode pn2 // why?
+          |
             n1 = pn1.getPreUpdateNode() and
             n2 = pn2.getPreUpdateNode()
           )
         )
       )
     }
 
-    /**
-     * Holds if data can flow from `fromNode` to `toNode` because they are the post-update
-     * nodes of some function output and input respectively, where the output and input
-     * are aliases. A typical example is a function returning `this`, implementing a fluent
-     * interface.
-     */
-    private predicate reverseStepThroughInputOutputAlias(
-      PostUpdateNode fromNode, PostUpdateNode toNode, string model
-    ) {
-      exists(Node fromPre, Node toPre |
-        fromPre = fromNode.getPreUpdateNode() and
-        toPre = toNode.getPreUpdateNode()
-      |
-        exists(DataFlowCall c |
-          // Does the language-specific simpleLocalFlowStep already model flow
-          // from function input to output?
-          fromPre = getAnOutNode(c, _) and
-          toPre.(ArgNode).argumentOf(c, _) and
-          simpleLocalFlowStep(toPre.(ArgNode), fromPre, model)
-        )
-        or
-        argumentValueFlowsThrough(toPre, TReadStepTypesNone(), fromPre, model)
-      )
-    }
-
     cached
-    predicate simpleLocalFlowStepExt(Node node1, Node node2, string model) {
-      simpleLocalFlowStep(node1, node2, model) or
-      reverseStepThroughInputOutputAlias(node1, node2, model)
+    predicate simpleLocalFlowStepCached(Node node1, Node node2, string model) {
+      simpleLocalFlowStep(node1, node2, model)
     }
 
     cached
@@ -2016,7 +1991,7 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
     newtype TNodeEx =
       TNodeNormal(Node n) or
       TNodeImplicitRead(Node n) or
-      TNodeReverse(Node n, Boolean b)
+      TNodeReverse(Node n, Boolean allowFwdFlowOut)
 
     /**
      * Holds if data can flow in one local step from `node1` to `node2`.
@@ -2026,13 +2001,15 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
       exists(Node n1, Node n2 |
         node1.asNode() = n1 and
         node2.asNode() = n2 and
-        simpleLocalFlowStepExt(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model)
+        simpleLocalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model)
       )
       or
+      // as soon as we take a reverse local step, forward out flow must be prohibited
+      // in order to prevent time travelling
       exists(Node n1, Node n2 |
         node1.asNodeReverse(_) = n1 and
         node2.asNodeReverse(false) = n2 and
-        simpleLocalFlowStepExt(pragma[only_bind_into](n2), pragma[only_bind_into](n1), model)
+        simpleLocalFlowStep(pragma[only_bind_into](n2), pragma[only_bind_into](n1), model)
       )
       or
       node1.asNode().(PostUpdateNode).getPreUpdateNode() = node2.asNodeReverse(true) and