Revert "Ruby: switch rb/sensitive-get-query back to using local flow"

This reverts commit fa58c51.

Author: alexrford

ruby/ql/lib/codeql/ruby/security/SensitiveGetQueryCustomizations.qll

@@ -0,0 +1,54 @@
+/**
+ * Provides default sources and sinks for reasoning about sensitive data sourced
+ * from the query string of a GET request, as well as extension points for
+ * adding your own.
+ */
+
+private import codeql.ruby.security.SensitiveActions
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow
+
+/**
+ * Provides default sources and sinks for reasoning about sensitive data sourced
+ * from the query string of a GET request, as well as extension points for
+ * adding your own.
+ */
+module SensitiveGetQuery {
+  /**
+   * A data flow source representing data sourced from the query string in a
+   * GET request handler.
+   */
+  abstract class Source extends DataFlow::Node {
+    /** Gets the request handler corresponding to this data source. */
+    abstract Http::Server::RequestHandler getHandler();
+  }
+
+  /**
+   * An access to data from the query string of a GET request as a data flow
+   * source.
+   */
+  private class RequestInputAccessSource extends Source instanceof Http::Server::RequestInputAccess {
+    private Http::Server::RequestHandler handler;
+
+    RequestInputAccessSource() {
+      handler = this.asExpr().getExpr().getEnclosingMethod() and
+      handler.getAnHttpMethod() = "get" and
+      this.getKind() = "parameter"
+    }
+
+    override Http::Server::RequestHandler getHandler() { result = handler }
+  }
+
+  /**
+   * A data flow sink suggesting a use of sensitive data.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /** A sensitive data node as a data flow sink. */
+  private class SensitiveNodeSink extends Sink instanceof SensitiveNode {
+    SensitiveNodeSink() {
+      // User names and other similar information is not sensitive in this context.
+      not this.getClassification() = SensitiveDataClassification::id()
+    }
+  }
+}

ruby/ql/lib/codeql/ruby/security/SensitiveGetQueryQuery.qll

@@ -0,0 +1,31 @@
+/**
+ * Provides a taint-tracking configuration for detecting flow of query string
+ * data to sensitive actions in GET query request handlers.
+ *
+ * Note, for performance reasons: only import this file if `Configuration` is
+ * needed, otherwise `SensitiveGetQueryCustomizations` should be imported
+ * instead.
+ */
+
+private import ruby
+private import codeql.ruby.TaintTracking
+
+/**
+ * Provides a taint-tracking configuration for detecting flow of query string
+ * data to sensitive actions in GET query request handlers.
+ */
+module SensitiveGetQuery {
+  import SensitiveGetQueryCustomizations::SensitiveGetQuery
+
+  /**
+   * A taint-tracking configuration for reasoning about use of sensitive data
+   * from a GET request query string.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "SensitiveGetQuery" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  }
+}

ruby/ql/src/queries/security/cwe-598/SensitiveGetQuery.ql

@@ -2,7 +2,7 @@
  * @name Sensitive data read from GET request
  * @description Placing sensitive data in a GET request increases the risk of
  *              the data being exposed to an attacker.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @security-severity 6.5
  * @precision high
@@ -12,30 +12,12 @@
  */
 
 import ruby
-import codeql.ruby.Concepts
+import DataFlow::PathGraph
+import codeql.ruby.security.SensitiveGetQueryQuery
 import codeql.ruby.security.SensitiveActions
 
-// Local flow augmented with flow through element references
-private predicate localFlowWithElementReference(DataFlow::LocalSourceNode src, DataFlow::Node to) {
-  src.flowsTo(to)
-  or
-  exists(DataFlow::Node midRecv, DataFlow::LocalSourceNode mid, Ast::ElementReference ref |
-    src.flowsTo(midRecv) and
-    midRecv.asExpr().getExpr() = ref.getReceiver() and
-    mid.asExpr().getExpr() = ref
-  |
-    localFlowWithElementReference(mid, to)
-  )
-}
-
-from
-  Http::Server::RequestHandler handler, Http::Server::RequestInputAccess input,
-  SensitiveNode sensitive
-where
-  handler.getAnHttpMethod() = "get" and
-  input.asExpr().getExpr().getEnclosingMethod() = handler and
-  input.getKind() = "parameter" and
-  localFlowWithElementReference(input, sensitive) and
-  not sensitive.getClassification() = SensitiveDataClassification::id()
-select input, "$@ for GET requests uses query parameter as sensitive data.", handler,
-  "Route handler"
+from DataFlow::PathNode source, DataFlow::PathNode sink, SensitiveGetQuery::Configuration config
+where config.hasFlowPath(source, sink)
+select source.getNode(), source, sink,
+  "$@ for GET requests uses query parameter as sensitive data.",
+  source.getNode().(SensitiveGetQuery::Source).getHandler(), "Route handler"

ruby/ql/test/query-tests/security/cwe-598/SensitiveGetQuery.expected

@@ -1 +1,13 @@
-| app/controllers/users_controller.rb:4:16:4:21 | call to params | $@ for GET requests uses query parameter as sensitive data. | app/controllers/users_controller.rb:3:3:6:5 | login_get | Route handler |
+edges
+| app/controllers/users_controller.rb:4:16:4:21 | call to params :  | app/controllers/users_controller.rb:4:16:4:32 | ...[...] |
+| app/controllers/users_controller.rb:4:16:4:21 | call to params :  | app/controllers/users_controller.rb:4:16:4:32 | ...[...] :  |
+| app/controllers/users_controller.rb:4:16:4:32 | ...[...] :  | app/controllers/users_controller.rb:5:42:5:49 | password |
+nodes
+| app/controllers/users_controller.rb:4:16:4:21 | call to params :  | semmle.label | call to params :  |
+| app/controllers/users_controller.rb:4:16:4:32 | ...[...] | semmle.label | ...[...] |
+| app/controllers/users_controller.rb:4:16:4:32 | ...[...] :  | semmle.label | ...[...] :  |
+| app/controllers/users_controller.rb:5:42:5:49 | password | semmle.label | password |
+subpaths
+#select
+| app/controllers/users_controller.rb:4:16:4:21 | call to params | app/controllers/users_controller.rb:4:16:4:21 | call to params :  | app/controllers/users_controller.rb:4:16:4:32 | ...[...] | $@ for GET requests uses query parameter as sensitive data. | app/controllers/users_controller.rb:3:3:6:5 | login_get | Route handler |
+| app/controllers/users_controller.rb:4:16:4:21 | call to params | app/controllers/users_controller.rb:4:16:4:21 | call to params :  | app/controllers/users_controller.rb:5:42:5:49 | password | $@ for GET requests uses query parameter as sensitive data. | app/controllers/users_controller.rb:3:3:6:5 | login_get | Route handler |