Merge pull request #16110 from hvitved/dataflow/param-flow-no-expects…

…-content Data flow: Block flow at `expectsContents` nodes in `parameterValueFlow`
github · Apr 9, 2024 · 5f8eb7b · 5f8eb7b
2 parents 1048cf7 + 7871fb8
commit 5f8eb7b
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 28 deletions.
diff --git a/ruby/ql/test/library-tests/dataflow/regressions/Regressions.expected b/ruby/ql/test/library-tests/dataflow/regressions/Regressions.expected
diff --git a/ruby/ql/test/library-tests/dataflow/regressions/Regressions.ql b/ruby/ql/test/library-tests/dataflow/regressions/Regressions.ql
@@ -0,0 +1,39 @@
+private import codeql.ruby.dataflow.FlowSummary
+
+private class ReverseSummary extends SimpleSummarizedCallable {
+  ReverseSummary() { this = "reverse" }
+
+  override predicate propagatesFlow(string input, string output, boolean preservesValue) {
+    input = "Argument[self].WithElement[any]" and
+    output = "ReturnValue" and
+    preservesValue = true
+  }
+}
+
+private module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    source
+        .(DataFlow::PostUpdateNode)
+        .getPreUpdateNode()
+        .asExpr()
+        .getExpr()
+        .(MethodCall)
+        .getMethodName() = "reverse"
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodCall mc |
+      mc.getMethodName() = "sink" and
+      sink.asExpr().getExpr() = mc.getAnArgument()
+    )
+  }
+}
+
+/**
+ * This predicate should not have a result. We check that the flow summary for
+ * `reverse` does not get picked up by the `reverseStepThroughInputOutputAlias`
+ * logic in `DataFlowImplCommon.qll`.
+ */
+query predicate noReverseStepThroughInputOutputAlias(DataFlow::Node source, DataFlow::Node sink) {
+  DataFlow::Global<Config>::flow(source, sink)
+}
diff --git a/ruby/ql/test/library-tests/dataflow/regressions/regressions.rb b/ruby/ql/test/library-tests/dataflow/regressions/regressions.rb
@@ -0,0 +1,3 @@
+x = foo
+x.reverse.bar
+sink(x)
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
@@ -863,34 +863,37 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> {
          */
         pragma[nomagic]
         private predicate parameterValueFlowCand(ParamNode p, Node node, boolean read) {
-          p = node and
-          read = false
-          or
-          // local flow
-          exists(Node mid |
-            parameterValueFlowCand(p, mid, read) and
-            simpleLocalFlowStep(mid, node) and
-            validParameterAliasStep(mid, node)
-          )
-          or
-          // read
-          exists(Node mid |
-            parameterValueFlowCand(p, mid, false) and
-            readSet(mid, _, node) and
-            read = true
-          )
-          or
-          // flow through: no prior read
-          exists(ArgNode arg |
-            parameterValueFlowArgCand(p, arg, false) and
-            argumentValueFlowsThroughCand(arg, node, read)
-          )
-          or
-          // flow through: no read inside method
-          exists(ArgNode arg |
-            parameterValueFlowArgCand(p, arg, read) and
-            argumentValueFlowsThroughCand(arg, node, false)
-          )
+          (
+            p = node and
+            read = false
+            or
+            // local flow
+            exists(Node mid |
+              parameterValueFlowCand(p, mid, read) and
+              simpleLocalFlowStep(mid, node) and
+              validParameterAliasStep(mid, node)
+            )
+            or
+            // read
+            exists(Node mid |
+              parameterValueFlowCand(p, mid, false) and
+              readSet(mid, _, node) and
+              read = true
+            )
+            or
+            // flow through: no prior read
+            exists(ArgNode arg |
+              parameterValueFlowArgCand(p, arg, false) and
+              argumentValueFlowsThroughCand(arg, node, read)
+            )
+            or
+            // flow through: no read inside method
+            exists(ArgNode arg |
+              parameterValueFlowArgCand(p, arg, read) and
+              argumentValueFlowsThroughCand(arg, node, false)
+            )
+          ) and
+          not expectsContentCached(node, _)
         }
 
         pragma[nomagic]