Merge pull request #208 from github/action-controller-1

Model accesses to `ActionController` parameters via `params` method

ql/src/codeql_ruby/dataflow/RemoteFlowSources.qll

@@ -0,0 +1,35 @@
+/**
+ * Provides an extension point for for modeling user-controlled data.
+ * Such data is often used as data-flow sources in security queries.
+ */
+
+private import codeql_ruby.dataflow.internal.DataFlowPublic as DataFlow
+
+/**
+ * A data flow source of remote user input.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `RemoteFlowSource::Range` instead.
+ */
+class RemoteFlowSource extends DataFlow::Node {
+  RemoteFlowSource::Range self;
+
+  RemoteFlowSource() { this = self }
+
+  /** Gets a string that describes the type of this remote flow source. */
+  string getSourceType() { result = self.getSourceType() }
+}
+
+/** Provides a class for modeling new sources of remote user input. */
+module RemoteFlowSource {
+  /**
+   * A data flow source of remote user input.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `RemoteFlowSource` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /** Gets a string that describes the type of this remote flow source. */
+    abstract string getSourceType();
+  }
+}

ql/src/codeql_ruby/frameworks/ActionController.qll

@@ -0,0 +1,86 @@
+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" }
+}
+
+/**
+ * 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" }
+}

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 |

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() }