wip

github · Aug 26, 2024 · 35caf3c · 35caf3c
1 parent 140d0e2
commit 35caf3c
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 28 deletions.
diff --git a/csharp/ql/src/Security Features/CWE-312/CleartextStorage.ql b/csharp/ql/src/Security Features/CWE-312/CleartextStorage.ql
@@ -17,6 +17,8 @@ import csharp
 import semmle.code.csharp.security.dataflow.CleartextStorageQuery
 import ClearTextStorage::PathGraph
 
+private predicate stats = ClearTextStorage::stageStats/10;
+
 from ClearTextStorage::PathNode source, ClearTextStorage::PathNode sink
 where ClearTextStorage::flowPath(source, sink)
 select sink.getNode(), source, sink, "This stores sensitive data returned by $@ as clear text.",

diff --git a/ruby/ql/test/library-tests/frameworks/action_controller/params-flow.expected b/ruby/ql/test/library-tests/frameworks/action_controller/params-flow.expected
@@ -56,28 +56,18 @@ edges
 | params_flow.rb:83:10:83:15 | call to params | params_flow.rb:83:10:83:27 | call to to_unsafe_h | provenance |  |
 | params_flow.rb:87:10:87:15 | call to params | params_flow.rb:87:10:87:30 | call to to_unsafe_hash | provenance |  |
 | params_flow.rb:91:10:91:15 | call to params | params_flow.rb:91:10:91:40 | call to transform_keys | provenance |  |
-| params_flow.rb:91:10:91:15 | call to params | params_flow.rb:91:10:91:40 | call to transform_keys [element] | provenance |  |
-| params_flow.rb:91:10:91:40 | call to transform_keys [element] | params_flow.rb:91:10:91:40 | call to transform_keys | provenance |  |
 | params_flow.rb:95:10:95:15 | call to params | params_flow.rb:95:10:95:41 | call to transform_keys! | provenance |  |
 | params_flow.rb:99:10:99:15 | call to params | params_flow.rb:99:10:99:42 | call to transform_values | provenance |  |
 | params_flow.rb:103:10:103:15 | call to params | params_flow.rb:103:10:103:43 | call to transform_values! | provenance |  |
 | params_flow.rb:107:10:107:15 | call to params | params_flow.rb:107:10:107:33 | call to values_at | provenance |  |
-| params_flow.rb:107:10:107:15 | call to params | params_flow.rb:107:10:107:33 | call to values_at [element 0] | provenance |  |
-| params_flow.rb:107:10:107:15 | call to params | params_flow.rb:107:10:107:33 | call to values_at [element 1] | provenance |  |
-| params_flow.rb:107:10:107:33 | call to values_at [element 0] | params_flow.rb:107:10:107:33 | call to values_at | provenance |  |
-| params_flow.rb:107:10:107:33 | call to values_at [element 1] | params_flow.rb:107:10:107:33 | call to values_at | provenance |  |
 | params_flow.rb:111:10:111:15 | call to params | params_flow.rb:111:10:111:29 | call to merge | provenance |  |
-| params_flow.rb:112:10:112:29 | call to merge [element 0] | params_flow.rb:112:10:112:29 | call to merge | provenance |  |
 | params_flow.rb:112:23:112:28 | call to params | params_flow.rb:112:10:112:29 | call to merge | provenance |  |
-| params_flow.rb:112:23:112:28 | call to params | params_flow.rb:112:10:112:29 | call to merge [element 0] | provenance |  |
 | params_flow.rb:116:10:116:15 | call to params | params_flow.rb:116:10:116:37 | call to reverse_merge | provenance |  |
 | params_flow.rb:117:31:117:36 | call to params | params_flow.rb:117:10:117:37 | call to reverse_merge | provenance |  |
 | params_flow.rb:121:10:121:15 | call to params | params_flow.rb:121:10:121:43 | call to with_defaults | provenance |  |
 | params_flow.rb:122:31:122:36 | call to params | params_flow.rb:122:10:122:37 | call to with_defaults | provenance |  |
 | params_flow.rb:126:10:126:15 | call to params | params_flow.rb:126:10:126:30 | call to merge! | provenance |  |
-| params_flow.rb:127:10:127:30 | call to merge! [element 0] | params_flow.rb:127:10:127:30 | call to merge! | provenance |  |
 | params_flow.rb:127:24:127:29 | call to params | params_flow.rb:127:10:127:30 | call to merge! | provenance |  |
-| params_flow.rb:127:24:127:29 | call to params | params_flow.rb:127:10:127:30 | call to merge! [element 0] | provenance |  |
 | params_flow.rb:130:5:130:5 | [post] p | params_flow.rb:131:10:131:10 | p | provenance |  |
 | params_flow.rb:130:5:130:5 | [post] p [element 0] | params_flow.rb:131:10:131:10 | p | provenance |  |
 | params_flow.rb:130:14:130:19 | call to params | params_flow.rb:130:5:130:5 | [post] p | provenance |  |
@@ -199,7 +189,6 @@ nodes
 | params_flow.rb:87:10:87:30 | call to to_unsafe_hash | semmle.label | call to to_unsafe_hash |
 | params_flow.rb:91:10:91:15 | call to params | semmle.label | call to params |
 | params_flow.rb:91:10:91:40 | call to transform_keys | semmle.label | call to transform_keys |
-| params_flow.rb:91:10:91:40 | call to transform_keys [element] | semmle.label | call to transform_keys [element] |
 | params_flow.rb:95:10:95:15 | call to params | semmle.label | call to params |
 | params_flow.rb:95:10:95:41 | call to transform_keys! | semmle.label | call to transform_keys! |
 | params_flow.rb:99:10:99:15 | call to params | semmle.label | call to params |
@@ -208,12 +197,9 @@ nodes
 | params_flow.rb:103:10:103:43 | call to transform_values! | semmle.label | call to transform_values! |
 | params_flow.rb:107:10:107:15 | call to params | semmle.label | call to params |
 | params_flow.rb:107:10:107:33 | call to values_at | semmle.label | call to values_at |
-| params_flow.rb:107:10:107:33 | call to values_at [element 0] | semmle.label | call to values_at [element 0] |
-| params_flow.rb:107:10:107:33 | call to values_at [element 1] | semmle.label | call to values_at [element 1] |
 | params_flow.rb:111:10:111:15 | call to params | semmle.label | call to params |
 | params_flow.rb:111:10:111:29 | call to merge | semmle.label | call to merge |
 | params_flow.rb:112:10:112:29 | call to merge | semmle.label | call to merge |
-| params_flow.rb:112:10:112:29 | call to merge [element 0] | semmle.label | call to merge [element 0] |
 | params_flow.rb:112:23:112:28 | call to params | semmle.label | call to params |
 | params_flow.rb:116:10:116:15 | call to params | semmle.label | call to params |
 | params_flow.rb:116:10:116:37 | call to reverse_merge | semmle.label | call to reverse_merge |
@@ -226,7 +212,6 @@ nodes
 | params_flow.rb:126:10:126:15 | call to params | semmle.label | call to params |
 | params_flow.rb:126:10:126:30 | call to merge! | semmle.label | call to merge! |
 | params_flow.rb:127:10:127:30 | call to merge! | semmle.label | call to merge! |
-| params_flow.rb:127:10:127:30 | call to merge! [element 0] | semmle.label | call to merge! [element 0] |
 | params_flow.rb:127:24:127:29 | call to params | semmle.label | call to params |
 | params_flow.rb:130:5:130:5 | [post] p | semmle.label | [post] p |
 | params_flow.rb:130:5:130:5 | [post] p [element 0] | semmle.label | [post] p [element 0] |

diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@@ -356,10 +356,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
     }
 
     pragma[nomagic]
-    private predicate readSetEx(NodeEx node1, ContentSet c, NodeEx node2) {
-      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) and
-      stepFilter(node1, node2)
-      or
+    private predicate implicitReadStep(NodeEx node1, ContentSet c, NodeEx node2) {
       exists(Node n |
         node2.isImplicitReadNode(n) and
         Config::allowImplicitRead(n, c)
@@ -371,6 +368,14 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
       )
     }
 
+    pragma[nomagic]
+    private predicate readSetEx(NodeEx node1, ContentSet c, NodeEx node2) {
+      readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) and
+      stepFilter(node1, node2)
+      or
+      implicitReadStep(node1, c, node2)
+    }
+
     // inline to reduce fan-out via `getAReadContent`
     bindingset[c]
     private predicate read(NodeEx node1, Content c, NodeEx node2) {
@@ -2484,14 +2489,26 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
             abstract PathNodeImpl getASuccessorImpl(string label);
 
+            pragma[nomagic]
+            private PathNodeImpl getAnImplicitReadSuccessor(string label) {
+              exists(NodeEx readTarget |
+                implicitReadStep(this.getNodeEx(), _, readTarget) and
+                result = this.getASuccessorImpl(label) and
+                result.getNodeEx() = [readTarget, toNormalSinkNodeEx(readTarget)]
+              )
+            }
+
             private PathNodeImpl getASuccessorIfHidden(string label) {
               this.isHidden() and
               result = this.getASuccessorImpl(label)
+              or
+              result = this.getAnImplicitReadSuccessor(label)
             }
 
             private PathNodeImpl getASuccessorFromNonHidden(string label) {
               result = this.getASuccessorImpl(label) and
-              not this.isHidden()
+              not this.isHidden() and
+              not result = this.getAnImplicitReadSuccessor(label)
               or
               exists(string l1, string l2 |
                 result = this.getASuccessorFromNonHidden(l1).getASuccessorIfHidden(l2) and
@@ -2500,7 +2517,8 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
             }
 
             final PathNodeImpl getANonHiddenSuccessor(string label) {
-              result = this.getASuccessorFromNonHidden(label) and not result.isHidden()
+              result = this.getASuccessorFromNonHidden(label) and
+              not result.isHidden()
             }
 
             predicate isHidden() {
@@ -3423,6 +3441,12 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
         }
       }
 
+      private predicate actualReadStep(NodeEx node1, NodeEx node2) {
+        Stage2::readStepCand(node1, _, node2) //and
+        // not node1.isImplicitReadNode(_) //and
+        // not node2.isImplicitReadNode(_)
+      }
+
       /**
        * Holds if `node` can be the first node in a maximal subsequence of local
        * flow steps in a dataflow path.
@@ -3444,7 +3468,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
           or
           Stage2::storeStepCand(_, _, _, node, _, _)
           or
-          Stage2::readStepCand(_, _, node)
+          actualReadStep(_, node)
           or
           node instanceof FlowCheckNode
           or
@@ -3461,12 +3485,17 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
        */
       private predicate localFlowExit(NodeEx node, FlowState state) {
         exists(NodeEx next | Stage2::revFlow(next, state) |
-          jumpStepEx(node, next) or
-          additionalJumpStep(node, next, _) or
-          flowIntoCallNodeCand2(_, node, next, _) or
-          flowOutOfCallNodeCand2(_, node, _, next, _) or
-          Stage2::storeStepCand(node, _, _, next, _, _) or
-          Stage2::readStepCand(node, _, next)
+          jumpStepEx(node, next)
+          or
+          additionalJumpStep(node, next, _)
+          or
+          flowIntoCallNodeCand2(_, node, next, _)
+          or
+          flowOutOfCallNodeCand2(_, node, _, next, _)
+          or
+          Stage2::storeStepCand(node, _, _, next, _, _)
+          or
+          actualReadStep(node, next)
         )
         or
         exists(NodeEx next, FlowState s | Stage2::revFlow(next, s) |