From 5941eb2be413a52eaff80eb38f2599eaa23a475c Mon Sep 17 00:00:00 2001
From: Alex Ford <alexrford@github.com>
Date: Tue, 15 Jun 2021 12:48:03 +0100
Subject: [PATCH] model some ActionController user input sources (params)

---
 .../frameworks/ActionController.qll           | 87 +++++++++++++++++++
 .../frameworks/ActionController.expected      | 18 ++++
 .../frameworks/ActionController.ql            |  8 ++
 3 files changed, 113 insertions(+)
 create mode 100644 ql/src/codeql_ruby/frameworks/ActionController.qll
 create mode 100644 ql/test/library-tests/frameworks/ActionController.expected
 create mode 100644 ql/test/library-tests/frameworks/ActionController.ql

diff --git a/ql/src/codeql_ruby/frameworks/ActionController.qll b/ql/src/codeql_ruby/frameworks/ActionController.qll
new file mode 100644
index 00000000000..14cbd7b4ecc
--- /dev/null
+++ b/ql/src/codeql_ruby/frameworks/ActionController.qll
@@ -0,0 +1,87 @@
+private import codeql_ruby.AST
+private import codeql_ruby.Concepts
+private import codeql_ruby.controlflow.CfgNodes
+private import codeql_ruby.DataFlow
+private import codeql_ruby.dataflow.RemoteFlowSources
+private import codeql_ruby.ast.internal.Module
+
+private class ActionControllerBaseAccess extends ConstantReadAccess {
+  ActionControllerBaseAccess() {
+    this.getName() = "Base" and
+    this.getScopeExpr().(ConstantAccess).getName() = "ActionController"
+  }
+}
+
+// ApplicationController extends ActionController::Base, but we
+// treat it separately in case the ApplicationController definition
+// is not in the database
+private class ApplicationControllerAccess extends ConstantReadAccess {
+  ApplicationControllerAccess() { this.getName() = "ApplicationController" }
+}
+
+// TODO: Less clumsy name for this?
+/**
+ * A `ClassDeclaration` for a class that extends `ActionController::Base`.
+ * For example,
+ *
+ * ```rb
+ * class FooController < ActionController::Base
+ *   def delete_handler
+ *     uid = params[:id]
+ *     User.delete_all("id = ?", uid)
+ *   end
+ * end
+ * ```
+ */
+class ActionControllerControllerClass extends ClassDeclaration {
+  ActionControllerControllerClass() {
+    // class FooController < ActionController::Base
+    this.getSuperclassExpr() instanceof ActionControllerBaseAccess
+    or
+    // class FooController < ApplicationController
+    this.getSuperclassExpr() instanceof ApplicationControllerAccess
+    or
+    // class BarController < FooController
+    exists(ActionControllerControllerClass other |
+      other.getModule() = resolveScopeExpr(this.getSuperclassExpr())
+    )
+  }
+}
+
+/**
+ * A call to the `params` method within the context of an
+ * `ActionControllerControllerClass`. For example, the `params` call in:
+ *
+ * ```rb
+ * class FooController < ActionController::Base
+ *   def delete_handler
+ *     uid = params[:id]
+ *     User.delete_all("id = ?", uid)
+ *   end
+ * end
+ * ```
+ */
+class ActionControllerParamsCall extends MethodCall {
+  private ActionControllerControllerClass controllerClass;
+
+  ActionControllerParamsCall() {
+    this.getMethodName() = "params" and
+    this.getReceiver() instanceof Self and
+    this.getEnclosingModule() = controllerClass
+  }
+
+  ActionControllerControllerClass getControllerClass() { result = controllerClass }
+}
+
+/**
+ * A `RemoteFlowSource::Range` to represent accessing the Action Controller
+ * parameters available to a controller via the `params` method.
+ */
+class ActionControllerParamsSource extends RemoteFlowSource::Range {
+  ActionControllerParamsCall call;
+
+  ActionControllerParamsSource() { this.asExpr().getExpr() = call }
+
+  // TODO: what to use here?
+  override string getSourceType() { result = "ActionController::Metal#params" }
+}
diff --git a/ql/test/library-tests/frameworks/ActionController.expected b/ql/test/library-tests/frameworks/ActionController.expected
new file mode 100644
index 00000000000..fec86cbdb8e
--- /dev/null
+++ b/ql/test/library-tests/frameworks/ActionController.expected
@@ -0,0 +1,18 @@
+actionControllerControllerClasses
+| ActiveRecordInjection.rb:12:1:34:3 | FooController |
+| ActiveRecordInjection.rb:37:1:48:3 | BarController |
+| ActiveRecordInjection.rb:50:1:51:3 | BazController |
+actionControllerParamsCalls
+| ActiveRecordInjection.rb:19:30:19:35 | call to params |
+| ActiveRecordInjection.rb:22:29:22:34 | call to params |
+| ActiveRecordInjection.rb:25:31:25:36 | call to params |
+| ActiveRecordInjection.rb:29:20:29:25 | call to params |
+| ActiveRecordInjection.rb:32:48:32:53 | call to params |
+| ActiveRecordInjection.rb:40:10:40:15 | call to params |
+actionControllerParamsSources
+| ActiveRecordInjection.rb:19:30:19:35 | call to params |
+| ActiveRecordInjection.rb:22:29:22:34 | call to params |
+| ActiveRecordInjection.rb:25:31:25:36 | call to params |
+| ActiveRecordInjection.rb:29:20:29:25 | call to params |
+| ActiveRecordInjection.rb:32:48:32:53 | call to params |
+| ActiveRecordInjection.rb:40:10:40:15 | call to params |
diff --git a/ql/test/library-tests/frameworks/ActionController.ql b/ql/test/library-tests/frameworks/ActionController.ql
new file mode 100644
index 00000000000..8b931ee75d5
--- /dev/null
+++ b/ql/test/library-tests/frameworks/ActionController.ql
@@ -0,0 +1,8 @@
+import codeql_ruby.controlflow.CfgNodes
+import codeql_ruby.frameworks.ActionController
+
+query predicate actionControllerControllerClasses(ActionControllerControllerClass cls) { any() }
+
+query predicate actionControllerParamsCalls(ActionControllerParamsCall call) { any() }
+
+query predicate actionControllerParamsSources(ActionControllerParamsSource source) { any() }