Ruby: port part of ActionController model

ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll

@@ -24,7 +24,7 @@ deprecated class ParamsCall = Rails::ParamsCall;
 deprecated class CookiesCall = Rails::CookiesCall;
 
 /**
- * A `ClassDeclaration` for a class that extends `ActionController::Base`.
+ * A class that extends `ActionController::Base`.
  * For example,
  *
  * ```rb
@@ -36,26 +36,59 @@ deprecated class CookiesCall = Rails::CookiesCall;
  * end
  * ```
  */
-class ActionControllerControllerClass extends ClassDeclaration {
-  ActionControllerControllerClass() {
-    this.getSuperclassExpr() =
+class ActionControllerClass extends DataFlow::ClassNode {
+  ActionControllerClass() {
+    this =
       [
-        API::getTopLevelMember("ActionController").getMember("Base"),
+        DataFlow::getConst("ActionController").getConst("Base"),
         // In Rails applications `ApplicationController` typically extends `ActionController::Base`, but we
         // treat it separately in case the `ApplicationController` definition is not in the database.
-        API::getTopLevelMember("ApplicationController"),
+        DataFlow::getConst("ApplicationController"),
         // ActionController::Metal technically doesn't contain all of the
         // methods available in Base, such as those for rendering views.
         // However we prefer to be over-sensitive in this case in order to find
         // more results.
-        API::getTopLevelMember("ActionController").getMember("Metal")
-      ].getASubclass().getAValueReachableFromSource().asExpr().getExpr()
+        DataFlow::getConst("ActionController").getConst("Metal")
+      ].getADescendentModule()
   }
 
   /**
    * Gets a `ActionControllerActionMethod` defined in this class.
    */
-  ActionControllerActionMethod getAnAction() { result = this.getAMethod() }
+  ActionControllerActionMethod getAnAction() { result = this.getAnInstanceMethod().asMethod() }
+
+  /**
+   * Gets a `self` that possibly refers to an instance of this class.
+   */
+  DataFlow::LocalSourceNode getSelf() {
+    result = this.getAnInstanceSelf()
+    or
+    // Include the module-level `self` to recover some cases where a block at the module level
+    // is invoked with an instance as the `self`, which we currently can't model directly.
+    // Concretely this happens in the block passed to `rescue_from`.
+    // TODO: revisit when we have better infrastructure for handling self in a block
+    result = this.getModuleLevelSelf()
+  }
+}
+
+private DataFlow::LocalSourceNode actionControllerInstance() {
+  result = any(ActionControllerClass cls).getSelf()
+}
+
+/**
+ * DEPRECATED. Use `ActionControllerClass` instead.
+ *
+ * A `ClassDeclaration` corresponding to an `ActionControllerClass`.
+ */
+deprecated class ActionControllerControllerClass extends ClassDeclaration {
+  ActionControllerControllerClass() { this = any(ActionControllerClass cls).getADeclaration() }
+
+  /**
+   * Gets a `ActionControllerActionMethod` defined in this class.
+   */
+  ActionControllerActionMethod getAnAction() {
+    result = this.getAMethod().(Method) and result.isPrivate()
+  }
 }
 
 /**
@@ -63,9 +96,11 @@ class ActionControllerControllerClass extends ClassDeclaration {
  * This may be the target of a route handler, if such a route is defined.
  */
 class ActionControllerActionMethod extends Method, Http::Server::RequestHandler::Range {
-  private ActionControllerControllerClass controllerClass;
+  private ActionControllerClass controllerClass;
 
-  ActionControllerActionMethod() { this = controllerClass.getAMethod() and not this.isPrivate() }
+  ActionControllerActionMethod() {
+    this = controllerClass.getAnInstanceMethod().asMethod() and not this.isPrivate()
+  }
 
   /**
    * Establishes a mapping between a method within the file
@@ -91,7 +126,7 @@ class ActionControllerActionMethod extends Method, Http::Server::RequestHandler:
   /**
    * Gets the controller class containing this method.
    */
-  ActionControllerControllerClass getControllerClass() {
+  ActionControllerClass getControllerClass() {
     // TODO: model the implicit render call when a path through the method does
     // not end at an explicit render or redirect
     result = controllerClass
@@ -102,37 +137,23 @@ class ActionControllerActionMethod extends Method, Http::Server::RequestHandler:
    * May return multiple results.
    */
   ActionDispatch::Routing::Route getARoute() {
-    exists(string name |
+    exists(string name, DataFlow::MethodNode m |
       isRoute(result, name, controllerClass) and
-      isActionControllerMethod(this, name, controllerClass)
+      m = controllerClass.getInstanceMethod(name) and
+      this = m.asMethod()
     )
   }
 }
 
 pragma[nomagic]
 private predicate isRoute(
-  ActionDispatch::Routing::Route route, string name, ActionControllerControllerClass controllerClass
+  ActionDispatch::Routing::Route route, string name, ActionControllerClass controllerClass
 ) {
   route.getController() + "_controller" =
-    ActionDispatch::Routing::underscore(controllerClass.getAQualifiedName()) and
+    ActionDispatch::Routing::underscore(controllerClass.getQualifiedName()) and
   name = route.getAction()
 }
 
-// A method call with a `self` receiver from within a controller class
-private class ActionControllerContextCall extends MethodCall {
-  private ActionControllerControllerClass controllerClass;
-
-  ActionControllerContextCall() {
-    this.getReceiver() instanceof SelfVariableAccess and
-    this.getEnclosingModule() = controllerClass
-  }
-
-  /**
-   * Gets the controller class containing this method.
-   */
-  ActionControllerControllerClass getControllerClass() { result = controllerClass }
-}
-
 /**
  * A `RemoteFlowSource::Range` to represent accessing the
  * ActionController parameters available via the `params` method.
@@ -158,13 +179,17 @@ class CookiesSource extends Http::Server::RequestInputAccess::Range {
 }
 
 /** A call to `cookies` from within a controller. */
-private class ActionControllerCookiesCall extends ActionControllerContextCall, CookiesCallImpl {
-  ActionControllerCookiesCall() { this.getMethodName() = "cookies" }
+private class ActionControllerCookiesCall extends CookiesCallImpl {
+  ActionControllerCookiesCall() {
+    this = actionControllerInstance().getAMethodCall("cookies").asExpr().getExpr()
+  }
 }
 
 /** A call to `params` from within a controller. */
-private class ActionControllerParamsCall extends ActionControllerContextCall, ParamsCallImpl {
-  ActionControllerParamsCall() { this.getMethodName() = "params" }
+private class ActionControllerParamsCall extends ParamsCallImpl {
+  ActionControllerParamsCall() {
+    this = actionControllerInstance().getAMethodCall("params").asExpr().getExpr()
+  }
 }
 
 /** Modeling for `ActionDispatch::Request`. */
@@ -174,10 +199,7 @@ private module Request {
    * `ActionDispatch::Request`.
    */
   private class RequestNode extends DataFlow::CallNode {
-    RequestNode() {
-      this.asExpr().getExpr() instanceof ActionControllerContextCall and
-      this.getMethodName() = "request"
-    }
+    RequestNode() { this = actionControllerInstance().getAMethodCall("request") }
   }
 
   /**
@@ -290,8 +312,7 @@ private module Request {
    */
   private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
     EnvHttpAccess() {
-      any(EnvCall c).(DataFlow::LocalSourceNode).flowsTo(this.getReceiver()) and
-      this.getMethodName() = "[]" and
+      this = any(EnvCall c).getAMethodCall("[]") and
       this.getArgument(0).getConstantValue().getString().regexpMatch("^HTTP_.+")
     }
 
@@ -302,21 +323,32 @@ private module Request {
 }
 
 /** A call to `render` from within a controller. */
-private class ActionControllerRenderCall extends ActionControllerContextCall, RenderCallImpl {
-  ActionControllerRenderCall() { this.getMethodName() = "render" }
+private class ActionControllerRenderCall extends RenderCallImpl {
+  ActionControllerRenderCall() {
+    this = actionControllerInstance().getAMethodCall("render").asExpr().getExpr()
+  }
 }
 
 /** A call to `render_to` from within a controller. */
-private class ActionControllerRenderToCall extends ActionControllerContextCall, RenderToCallImpl {
-  ActionControllerRenderToCall() { this.getMethodName() = ["render_to_body", "render_to_string"] }
+private class ActionControllerRenderToCall extends RenderToCallImpl {
+  ActionControllerRenderToCall() {
+    this =
+      actionControllerInstance()
+          .getAMethodCall(["render_to_body", "render_to_string"])
+          .asExpr()
+          .getExpr()
+  }
 }
 
 /** A call to `html_escape` from within a controller. */
 private class ActionControllerHtmlEscapeCall extends HtmlEscapeCallImpl {
   ActionControllerHtmlEscapeCall() {
     // "h" is aliased to "html_escape" in ActiveSupport
-    this.getMethodName() = ["html_escape", "html_escape_once", "h", "sanitize"] and
-    this.getEnclosingModule() instanceof ActionControllerControllerClass
+    this =
+      actionControllerInstance()
+          .getAMethodCall(["html_escape", "html_escape_once", "h", "sanitize"])
+          .asExpr()
+          .getExpr()
   }
 }
 
@@ -324,9 +356,16 @@ private class ActionControllerHtmlEscapeCall extends HtmlEscapeCallImpl {
  * A call to the `redirect_to` method, used in an action to redirect to a
  * specific URL/path or to a different action in this controller.
  */
-class RedirectToCall extends ActionControllerContextCall {
+class RedirectToCall extends MethodCall {
+  private ActionControllerClass controller;
+
   RedirectToCall() {
-    this.getMethodName() = ["redirect_to", "redirect_back", "redirect_back_or_to"]
+    this =
+      controller
+          .getSelf()
+          .getAMethodCall(["redirect_to", "redirect_back", "redirect_back_or_to"])
+          .asExpr()
+          .getExpr()
   }
 
   /** Gets the `Expr` representing the URL to redirect to, if any */
@@ -338,8 +377,10 @@ class RedirectToCall extends ActionControllerContextCall {
 
   /** Gets the `ActionControllerActionMethod` to redirect to, if any */
   ActionControllerActionMethod getRedirectActionMethod() {
-    this.getKeywordArgument("action").getConstantValue().isStringlikeValue(result.getName()) and
-    result.getEnclosingModule() = this.getControllerClass()
+    exists(string name |
+      this.getKeywordArgument("action").getConstantValue().isStringlikeValue(name) and
+      result = controller.getInstanceMethod(name).asMethod()
+    )
   }
 
   /**
@@ -373,18 +414,8 @@ class ActionControllerRedirectResponse extends Http::Server::HttpRedirectRespons
 }
 
 pragma[nomagic]
-private predicate isActionControllerMethod(Method m, string name, ActionControllerControllerClass c) {
-  m.getName() = name and
-  m.getEnclosingModule() = c
-}
-
-pragma[nomagic]
-private predicate actionControllerHasHelperMethodCall(ActionControllerControllerClass c, string name) {
-  exists(MethodCall mc |
-    mc.getMethodName() = "helper_method" and
-    mc.getAnArgument().getConstantValue().isStringlikeValue(name) and
-    mc.getEnclosingModule() = c
-  )
+private predicate actionControllerHasHelperMethodCall(ActionControllerClass c, string name) {
+  c.getAModuleLevelCall("helper_method").getArgument(_).getConstantValue().isStringlikeValue(name)
 }
 
 /**
@@ -404,27 +435,28 @@ private predicate actionControllerHasHelperMethodCall(ActionControllerController
  * See also https://api.rubyonrails.org/classes/AbstractController/Helpers/ClassMethods.html#method-i-helper_method
  */
 class ActionControllerHelperMethod extends Method {
-  private ActionControllerControllerClass controllerClass;
+  private ActionControllerClass controllerClass;
 
   ActionControllerHelperMethod() {
-    exists(string name |
-      isActionControllerMethod(this, name, controllerClass) and
-      actionControllerHasHelperMethodCall(controllerClass, name)
+    exists(DataFlow::MethodNode m, string name |
+      m = controllerClass.getInstanceMethod(name) and
+      actionControllerHasHelperMethodCall(controllerClass, name) and
+      this = m.asMethod()
     )
   }
 
   /** Gets the class containing this helper method. */
-  ActionControllerControllerClass getControllerClass() { result = controllerClass }
+  ActionControllerClass getControllerClass() { result = controllerClass }
 }
 
 /**
- * Gets an `ActionControllerControllerClass` associated with the given `ErbFile`
+ * Gets an `ActionControllerClass` associated with the given `ErbFile`
  * according to Rails path conventions.
  * For instance, a template file at `app/views/foo/bar/baz.html.erb` will be
  * mapped to a controller class in `app/controllers/foo/bar/baz_controller.rb`,
  * if such a controller class exists.
  */
-ActionControllerControllerClass getAssociatedControllerClass(ErbFile f) {
+ActionControllerClass getAssociatedControllerClass(ErbFile f) {
   // There is a direct mapping from template file to controller class
   controllerTemplateFile(result, f)
   or
@@ -448,7 +480,7 @@ ActionControllerControllerClass getAssociatedControllerClass(ErbFile f) {
  * This handles mappings between controllers in `app/controllers/`, and
  * templates in `app/views/` and `app/views/layouts/`.
  */
-predicate controllerTemplateFile(ActionControllerControllerClass cls, ErbFile templateFile) {
+predicate controllerTemplateFile(ActionControllerClass cls, ErbFile templateFile) {
   exists(string templatesPath, string sourcePrefix, string subPath, string controllerPath |
     controllerPath = cls.getLocation().getFile().getRelativePath() and
     templatesPath = templateFile.getParentContainer().getRelativePath() and
@@ -484,16 +516,14 @@ class ActionControllerSkipForgeryProtectionCall extends CsrfProtectionSetting::R
 /**
  * A call to `protect_from_forgery`.
  */
-private class ActionControllerProtectFromForgeryCall extends CsrfProtectionSetting::Range {
-  private ActionControllerContextCall callExpr;
-
+private class ActionControllerProtectFromForgeryCall extends CsrfProtectionSetting::Range,
+  DataFlow::CallNode {
   ActionControllerProtectFromForgeryCall() {
-    callExpr = this.asExpr().getExpr() and
-    callExpr.getMethodName() = "protect_from_forgery"
+    this = actionControllerInstance().getAMethodCall("protect_from_forgery")
   }
 
   private string getWithValueText() {
-    result = callExpr.getKeywordArgument("with").getConstantValue().getSymbol()
+    result = getKeywordArgument("with").getConstantValue().getSymbol()
   }
 
   // Calls without `with: :exception` can allow for bypassing CSRF protection
@@ -508,11 +538,7 @@ private class ActionControllerProtectFromForgeryCall extends CsrfProtectionSetti
  */
 private class SendFile extends FileSystemAccess::Range, DataFlow::CallNode {
   SendFile() {
-    this.getMethodName() = "send_file" and
-    (
-      this.asExpr().getExpr() instanceof ActionControllerContextCall or
-      this.getReceiver().asExpr().getExpr() instanceof Response::ResponseCall
-    )
+    this = [actionControllerInstance(), Response::response()].getAMethodCall("send_file")
   }
 
   override DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
@@ -522,21 +548,13 @@ private module ParamsSummaries {
   private import codeql.ruby.dataflow.FlowSummary
 
   /**
-   * An instance of `ActionController::Parameters`, including those returned
+   * Gets an instance of `ActionController::Parameters`, including those returned
    * from method calls on other instances.
    */
-  private class ParamsInstance extends DataFlow::Node {
-    ParamsInstance() {
-      this.asExpr().getExpr() instanceof Rails::ParamsCall
-      or
-      this =
-        any(DataFlow::CallNode call |
-          call.getReceiver() instanceof ParamsInstance and
-          call.getMethodName() = paramsMethodReturningParamsInstance()
-        )
-      or
-      exists(ParamsInstance prev | prev.(DataFlow::LocalSourceNode).flowsTo(this))
-    }
+  private DataFlow::LocalSourceNode paramsInstance() {
+    result.asExpr().getExpr() instanceof Rails::ParamsCall
+    or
+    result = paramsInstance().getAMethodCall(paramsMethodReturningParamsInstance())
   }
 
   /**
@@ -578,8 +596,7 @@ private module ParamsSummaries {
     MethodsReturningParamsInstanceSummary() { this = "ActionController::Parameters#<various>" }
 
     override MethodCall getACall() {
-      any(ParamsInstance i).asExpr().getExpr() = result.getReceiver() and
-      result.getMethodName() = methodReturnsTaintFromSelf()
+      result = paramsInstance().getAMethodCall(methodReturnsTaintFromSelf()).asExpr().getExpr()
     }
 
     override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
@@ -601,9 +618,8 @@ private module ParamsSummaries {
 
     override MethodCall getACall() {
       result.getMethodName() = ["merge", "reverse_merge", "with_defaults"] and
-      exists(ParamsInstance i |
-        i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
-      )
+      paramsInstance().getALocalUse().asExpr().getExpr() =
+        [result.getReceiver(), result.getArgument(0)]
     }
 
     override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
@@ -625,9 +641,8 @@ private module ParamsSummaries {
 
     override MethodCall getACall() {
       result.getMethodName() = ["merge!", "reverse_merge!", "with_defaults!"] and
-      exists(ParamsInstance i |
-        i.asExpr().getExpr() = [result.getReceiver(), result.getArgument(0)]
-      )
+      paramsInstance().getALocalUse().asExpr().getExpr() =
+        [result.getReceiver(), result.getArgument(0)]
     }
 
     override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
@@ -643,15 +658,12 @@ private module ParamsSummaries {
  * response.
  */
 private module Response {
-  class ResponseCall extends ActionControllerContextCall {
-    ResponseCall() { this.getMethodName() = "response" }
+  DataFlow::LocalSourceNode response() {
+    result = actionControllerInstance().getAMethodCall("response")
   }
 
   class BodyWrite extends DataFlow::CallNode, Http::Server::HttpResponse::Range {
-    BodyWrite() {
-      this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
-      this.getMethodName() = "body="
-    }
+    BodyWrite() { this = response().getAMethodCall("body=") }
 
     override DataFlow::Node getBody() { result = this.getArgument(0) }
 
@@ -661,10 +673,7 @@ private module Response {
   }
 
   class SendFileCall extends DataFlow::CallNode, Http::Server::HttpResponse::Range {
-    SendFileCall() {
-      this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
-      this.getMethodName() = "send_file"
-    }
+    SendFileCall() { this = response().getAMethodCall("send_file") }
 
     override DataFlow::Node getBody() { result = this.getArgument(0) }
 
@@ -677,19 +686,12 @@ private module Response {
     HeaderWrite() {
       // response.header[key] = val
       // response.headers[key] = val
-      exists(MethodCall headerCall |
-        headerCall.getMethodName() = ["header", "headers"] and
-        headerCall.getReceiver() instanceof ResponseCall
-      |
-        this.getReceiver().asExpr().getExpr() = headerCall and
-        this.getMethodName() = "[]="
-      )
+      this = response().getAMethodCall(["header", "headers"]).getAMethodCall("[]=")
       or
       // response.set_header(key) = val
       // response[header] = val
       // response.add_header(key, val)
-      this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
-      this.getMethodName() = ["set_header", "[]=", "add_header"]
+      this = response().getAMethodCall(["set_header", "[]=", "add_header"])
     }
 
     override string getName() {
@@ -702,12 +704,12 @@ private module Response {
   class SpecificHeaderWrite extends DataFlow::CallNode, Http::Server::HeaderWriteAccess::Range {
     SpecificHeaderWrite() {
       // response.<method> = val
-      this.getReceiver().asExpr().getExpr() instanceof ResponseCall and
-      this.getMethodName() =
-        [
-          "location=", "cache_control=", "_cache_control=", "etag=", "charset=", "content_type=",
-          "date=", "last_modified=", "weak_etag=", "strong_etag="
-        ]
+      this =
+        response()
+            .getAMethodCall([
+                "location=", "cache_control=", "_cache_control=", "etag=", "charset=",
+                "content_type=", "date=", "last_modified=", "weak_etag=", "strong_etag="
+              ])
     }
 
     override string getName() {

ruby/ql/src/experimental/weak-params/WeakParams.ql

@@ -17,18 +17,11 @@ import codeql.ruby.frameworks.ActionController
 import DataFlow::PathGraph
 
 /**
- * A call to `request` in an ActionController controller class.
+ * Gets a call to `request` in an ActionController controller class.
  * This probably refers to the incoming HTTP request object.
  */
-class ActionControllerRequest extends DataFlow::Node {
-  ActionControllerRequest() {
-    exists(DataFlow::CallNode c |
-      c.asExpr().getExpr().getEnclosingModule() instanceof ActionControllerControllerClass and
-      c.getMethodName() = "request"
-    |
-      c.flowsTo(this)
-    )
-  }
+DataFlow::LocalSourceNode request() {
+  result = any(ActionControllerClass cls).getSelf().getAMethodCall("request")
 }
 
 /**
@@ -36,9 +29,11 @@ class ActionControllerRequest extends DataFlow::Node {
  */
 class WeakParams extends DataFlow::CallNode {
   WeakParams() {
-    this.getReceiver() instanceof ActionControllerRequest and
-    this.getMethodName() =
-      ["path_parameters", "query_parameters", "request_parameters", "GET", "POST"]
+    this =
+      request()
+          .getAMethodCall([
+              "path_parameters", "query_parameters", "request_parameters", "GET", "POST"
+            ])
   }
 }
 

ruby/ql/test/library-tests/frameworks/ActionController.expected

@@ -1,17 +1,17 @@
 actionControllerControllerClasses
 | action_controller/input_access.rb:1:1:50:3 | UsersController |
-| action_controller/params_flow.rb:1:1:151:3 | MyController |
+| action_controller/params_flow.rb:1:1:153:3 | MyController |
+| action_controller/params_flow.rb:161:1:169:3 | Subclass |
 | active_record/ActiveRecord.rb:23:1:39:3 | FooController |
 | active_record/ActiveRecord.rb:41:1:64:3 | BarController |
 | active_record/ActiveRecord.rb:66:1:98:3 | BazController |
 | active_record/ActiveRecord.rb:100:1:108:3 | AnnotatedController |
-| active_storage/active_storage.rb:39:1:45:3 | PostsController |
 | app/controllers/comments_controller.rb:1:1:40:3 | CommentsController |
 | app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController |
 | app/controllers/photos_controller.rb:1:1:4:3 | PhotosController |
 | app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
 | app/controllers/tags_controller.rb:1:1:2:3 | TagsController |
-| app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController |
+| app/controllers/users/notifications_controller.rb:2:3:5:5 | Users::NotificationsController |
 actionControllerActionMethods
 | action_controller/input_access.rb:2:3:49:5 | index |
 | action_controller/params_flow.rb:2:3:4:5 | m1 |
@@ -47,6 +47,8 @@ actionControllerActionMethods
 | action_controller/params_flow.rb:125:3:132:5 | m30 |
 | action_controller/params_flow.rb:134:3:141:5 | m31 |
 | action_controller/params_flow.rb:143:3:150:5 | m32 |
+| action_controller/params_flow.rb:156:3:158:5 | m33 |
+| action_controller/params_flow.rb:162:3:164:5 | m34 |
 | active_record/ActiveRecord.rb:27:3:38:5 | some_request_handler |
 | active_record/ActiveRecord.rb:42:3:47:5 | some_other_request_handler |
 | active_record/ActiveRecord.rb:49:3:63:5 | safe_paths |
@@ -117,6 +119,9 @@ paramsCalls
 | action_controller/params_flow.rb:144:10:144:15 | call to params |
 | action_controller/params_flow.rb:145:32:145:37 | call to params |
 | action_controller/params_flow.rb:148:22:148:27 | call to params |
+| action_controller/params_flow.rb:157:10:157:15 | call to params |
+| action_controller/params_flow.rb:163:10:163:15 | call to params |
+| action_controller/params_flow.rb:167:10:167:15 | call to params |
 | action_mailer/mailer.rb:3:10:3:15 | call to params |
 | active_record/ActiveRecord.rb:28:30:28:35 | call to params |
 | active_record/ActiveRecord.rb:29:29:29:34 | call to params |
@@ -192,6 +197,9 @@ paramsSources
 | action_controller/params_flow.rb:144:10:144:15 | call to params |
 | action_controller/params_flow.rb:145:32:145:37 | call to params |
 | action_controller/params_flow.rb:148:22:148:27 | call to params |
+| action_controller/params_flow.rb:157:10:157:15 | call to params |
+| action_controller/params_flow.rb:163:10:163:15 | call to params |
+| action_controller/params_flow.rb:167:10:167:15 | call to params |
 | action_mailer/mailer.rb:3:10:3:15 | call to params |
 | active_record/ActiveRecord.rb:28:30:28:35 | call to params |
 | active_record/ActiveRecord.rb:29:29:29:34 | call to params |
@@ -306,6 +314,9 @@ httpInputAccesses
 | action_controller/params_flow.rb:144:10:144:15 | call to params | ActionController::Metal#params |
 | action_controller/params_flow.rb:145:32:145:37 | call to params | ActionController::Metal#params |
 | action_controller/params_flow.rb:148:22:148:27 | call to params | ActionController::Metal#params |
+| action_controller/params_flow.rb:157:10:157:15 | call to params | ActionController::Metal#params |
+| action_controller/params_flow.rb:163:10:163:15 | call to params | ActionController::Metal#params |
+| action_controller/params_flow.rb:167:10:167:15 | call to params | ActionController::Metal#params |
 | action_mailer/mailer.rb:3:10:3:15 | call to params | ActionController::Metal#params |
 | active_record/ActiveRecord.rb:28:30:28:35 | call to params | ActionController::Metal#params |
 | active_record/ActiveRecord.rb:29:29:29:34 | call to params | ActionController::Metal#params |

ruby/ql/test/library-tests/frameworks/ActionController.ql

@@ -5,7 +5,7 @@ private import codeql.ruby.frameworks.ActionView
 private import codeql.ruby.Concepts
 private import codeql.ruby.DataFlow
 
-query predicate actionControllerControllerClasses(ActionControllerControllerClass cls) { any() }
+query predicate actionControllerControllerClasses(ActionControllerClass cls) { any() }
 
 query predicate actionControllerActionMethods(ActionControllerActionMethod m) { any() }
 
@@ -25,11 +25,11 @@ query predicate redirectToCalls(RedirectToCall c) { any() }
 
 query predicate actionControllerHelperMethods(ActionControllerHelperMethod m) { any() }
 
-query predicate getAssociatedControllerClasses(ActionControllerControllerClass cls, ErbFile f) {
+query predicate getAssociatedControllerClasses(ActionControllerClass cls, ErbFile f) {
   cls = getAssociatedControllerClass(f)
 }
 
-query predicate controllerTemplateFiles(ActionControllerControllerClass cls, ErbFile templateFile) {
+query predicate controllerTemplateFiles(ActionControllerClass cls, ErbFile templateFile) {
   controllerTemplateFile(cls, templateFile)
 }
 

ruby/ql/test/library-tests/frameworks/action_controller/params-flow.expected

@@ -44,6 +44,9 @@ edges
 | params_flow.rb:145:32:145:37 | call to params :  | params_flow.rb:145:10:145:38 | call to with_defaults! |
 | params_flow.rb:148:5:148:5 | [post] p :  | params_flow.rb:149:10:149:10 | p |
 | params_flow.rb:148:22:148:27 | call to params :  | params_flow.rb:148:5:148:5 | [post] p :  |
+| params_flow.rb:157:10:157:15 | call to params :  | params_flow.rb:157:10:157:19 | ...[...] |
+| params_flow.rb:163:10:163:15 | call to params :  | params_flow.rb:163:10:163:19 | ...[...] |
+| params_flow.rb:167:10:167:15 | call to params :  | params_flow.rb:167:10:167:19 | ...[...] |
 nodes
 | params_flow.rb:3:10:3:15 | call to params :  | semmle.label | call to params :  |
 | params_flow.rb:3:10:3:19 | ...[...] | semmle.label | ...[...] |
@@ -130,6 +133,12 @@ nodes
 | params_flow.rb:148:5:148:5 | [post] p :  | semmle.label | [post] p :  |
 | params_flow.rb:148:22:148:27 | call to params :  | semmle.label | call to params :  |
 | params_flow.rb:149:10:149:10 | p | semmle.label | p |
+| params_flow.rb:157:10:157:15 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:157:10:157:19 | ...[...] | semmle.label | ...[...] |
+| params_flow.rb:163:10:163:15 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:163:10:163:19 | ...[...] | semmle.label | ...[...] |
+| params_flow.rb:167:10:167:15 | call to params :  | semmle.label | call to params :  |
+| params_flow.rb:167:10:167:19 | ...[...] | semmle.label | ...[...] |
 subpaths
 #select
 | params_flow.rb:3:10:3:19 | ...[...] | params_flow.rb:3:10:3:15 | call to params :  | params_flow.rb:3:10:3:19 | ...[...] | $@ | params_flow.rb:3:10:3:15 | call to params :  | call to params :  |
@@ -173,3 +182,6 @@ subpaths
 | params_flow.rb:144:10:144:44 | call to with_defaults! | params_flow.rb:144:10:144:15 | call to params :  | params_flow.rb:144:10:144:44 | call to with_defaults! | $@ | params_flow.rb:144:10:144:15 | call to params :  | call to params :  |
 | params_flow.rb:145:10:145:38 | call to with_defaults! | params_flow.rb:145:32:145:37 | call to params :  | params_flow.rb:145:10:145:38 | call to with_defaults! | $@ | params_flow.rb:145:32:145:37 | call to params :  | call to params :  |
 | params_flow.rb:149:10:149:10 | p | params_flow.rb:148:22:148:27 | call to params :  | params_flow.rb:149:10:149:10 | p | $@ | params_flow.rb:148:22:148:27 | call to params :  | call to params :  |
+| params_flow.rb:157:10:157:19 | ...[...] | params_flow.rb:157:10:157:15 | call to params :  | params_flow.rb:157:10:157:19 | ...[...] | $@ | params_flow.rb:157:10:157:15 | call to params :  | call to params :  |
+| params_flow.rb:163:10:163:19 | ...[...] | params_flow.rb:163:10:163:15 | call to params :  | params_flow.rb:163:10:163:19 | ...[...] | $@ | params_flow.rb:163:10:163:15 | call to params :  | call to params :  |
+| params_flow.rb:167:10:167:19 | ...[...] | params_flow.rb:167:10:167:15 | call to params :  | params_flow.rb:167:10:167:19 | ...[...] | $@ | params_flow.rb:167:10:167:15 | call to params :  | call to params :  |

ruby/ql/test/library-tests/frameworks/action_controller/params_flow.rb

@@ -148,4 +148,22 @@ class MyController < ActionController::Base
     p.with_defaults!(params)
     sink p # $hasTaintFlow
   end
+
+  include Mixin
+end
+
+module Mixin
+  def m33
+    sink params[:x] # $hasTaintFlow
+  end
+end
+
+class Subclass < MyController
+  def m34
+    sink params[:x] # $hasTaintFlow
+  end
+
+  rescue_from 'Foo::Bar' do |err|
+    sink params[:x] # $hasTaintFlow
+  end
 end