Shared: prevent use-use flow through implicit reads

github · Aug 20, 2024 · cb0fe40 · cb0fe40
1 parent 03c6aed
commit cb0fe40
Showing 1 changed file with 28 additions and 14 deletions.
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@@ -179,6 +179,11 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
       Node asNode() { this = TNodeNormal(result) }
 
+      /** 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, true)
+      }
+
       predicate isImplicitReadNode(Node n, boolean hasRead) { this = TNodeImplicitRead(n, hasRead) }
 
       ParameterNode asParamReturnNode() { this = TParamReturnNode(result, _) }
@@ -247,6 +252,16 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       ReturnKindExt getKind() { result = pos.getKind() }
     }
 
+    /** If `node` corresponds to a sink, gets the normal node for that sink. */
+    pragma[nomagic]
+    private NodeEx toNormalSinkNodeEx(NodeEx node) {
+      exists(Node n |
+        node.asNodeOrImplicitRead() = n and
+        (Config::isSink(n) or Config::isSink(n, _)) and
+        result.asNode() = n
+      )
+    }
+
     private predicate inBarrier(NodeEx node) {
       exists(Node n |
         node.asNode() = n and
@@ -266,7 +281,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     private predicate outBarrier(NodeEx node) {
       exists(Node n |
-        node.asNode() = n and
+        node.asNodeOrImplicitRead() = n and
         Config::isBarrierOut(n)
       |
         Config::isSink(n, _)
@@ -278,7 +293,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     pragma[nomagic]
     private predicate outBarrier(NodeEx node, FlowState state) {
       exists(Node n |
-        node.asNode() = n and
+        node.asNodeOrImplicitRead() = n and
         Config::isBarrierOut(n, state)
       |
         Config::isSink(n, state)
@@ -324,7 +339,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     pragma[nomagic]
     private predicate sinkNodeWithState(NodeEx node, FlowState state) {
-      Config::isSink(node.asNode(), state) and
+      Config::isSink(node.asNodeOrImplicitRead(), state) and
       not fullBarrier(node) and
       not stateBarrier(node, state)
     }
@@ -386,7 +401,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
      */
     private predicate additionalLocalFlowStep(NodeEx node1, NodeEx node2, string model) {
       exists(Node n1, Node n2 |
-        node1.asNode() = n1 and
+        node1.asNodeOrImplicitRead() = n1 and
         node2.asNode() = n2 and
         Config::isAdditionalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) and
         getNodeEnclosingCallable(n1) = getNodeEnclosingCallable(n2) and
@@ -395,8 +410,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       or
       exists(Node n |
         node1.isImplicitReadNode(n, true) and
-        node2.asNode() = n and
-        not fullBarrier(node2) and
+        node2.isImplicitReadNode(n, false) and
         model = ""
       )
     }
@@ -405,7 +419,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       NodeEx node1, FlowState s1, NodeEx node2, FlowState s2
     ) {
       exists(Node n1, Node n2 |
-        node1.asNode() = n1 and
+        node1.asNodeOrImplicitRead() = n1 and
         node2.asNode() = n2 and
         Config::isAdditionalFlowStep(pragma[only_bind_into](n1), s1, pragma[only_bind_into](n2), s2) and
         getNodeEnclosingCallable(n1) = getNodeEnclosingCallable(n2) and
@@ -431,7 +445,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
      */
     private predicate additionalJumpStep(NodeEx node1, NodeEx node2, string model) {
       exists(Node n1, Node n2 |
-        node1.asNode() = n1 and
+        node1.asNodeOrImplicitRead() = n1 and
         node2.asNode() = n2 and
         Config::isAdditionalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) and
         getNodeEnclosingCallable(n1) != getNodeEnclosingCallable(n2) and
@@ -442,7 +456,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     private predicate additionalJumpStateStep(NodeEx node1, FlowState s1, NodeEx node2, FlowState s2) {
       exists(Node n1, Node n2 |
-        node1.asNode() = n1 and
+        node1.asNodeOrImplicitRead() = n1 and
         node2.asNode() = n2 and
         Config::isAdditionalFlowStep(pragma[only_bind_into](n1), s1, pragma[only_bind_into](n2), s2) and
         getNodeEnclosingCallable(n1) != getNodeEnclosingCallable(n2) and
@@ -735,7 +749,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       additional predicate sinkNode(NodeEx node, FlowState state) {
         fwdFlow(node) and
         fwdFlowState(state) and
-        Config::isSink(node.asNode())
+        Config::isSink(node.asNodeOrImplicitRead())
         or
         fwdFlow(node) and
         fwdFlowState(state) and
@@ -1058,7 +1072,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
     private predicate sinkModel(NodeEx node, string model) {
       sinkNode(node, _) and
-      exists(Node n | n = node.asNode() |
+      exists(Node n | n = node.asNodeOrImplicitRead() |
         knownSinkModel(n, model)
         or
         not knownSinkModel(n, _) and model = ""
@@ -3852,7 +3866,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       TPathNodeSink(NodeEx node, FlowState state) {
         exists(PathNodeMid sink |
           sink.isAtSink(_) and
-          node = sink.getNodeEx() and
+          node = toNormalSinkNodeEx(sink.getNodeEx()) and
           state = sink.getState()
         )
       } or
@@ -4339,7 +4353,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
       PathNodeSink projectToSink(string model) {
         this.isAtSink(model) and
-        result.getNodeEx() = node and
+        result.getNodeEx() = toNormalSinkNodeEx(node) and
         result.getState() = state
       }
     }
@@ -5231,7 +5245,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       private predicate revSinkNode(NodeEx node, FlowState state) {
         sinkNodeWithState(node, state)
         or
-        Config::isSink(node.asNode()) and
+        Config::isSink(node.asNodeOrImplicitRead()) and
         relevantState(state) and
         not fullBarrier(node) and
         not stateBarrier(node, state)