Merge pull request #262 from github/action-view-1

Start modelling ActionView

ql/src/codeql_ruby/Frameworks.qll

@@ -4,3 +4,4 @@
 
 private import codeql_ruby.frameworks.ActionController
 private import codeql_ruby.frameworks.ActiveRecord
+private import codeql_ruby.frameworks.ActionView

ql/src/codeql_ruby/frameworks/ActionController.qll

@@ -4,6 +4,7 @@ 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 import ActionView
 
 private class ActionControllerBaseAccess extends ConstantReadAccess {
   ActionControllerBaseAccess() {
@@ -45,26 +46,53 @@ class ActionControllerControllerClass extends ClassDeclaration {
       other.getModule() = resolveScopeExpr(this.getSuperclassExpr())
     )
   }
+
+  /**
+   * Gets a `ActionControllerActionMethod` defined in this class.
+   */
+  ActionControllerActionMethod getAnAction() { result = this.getAMethod() }
 }
 
 /**
- * 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
- * ```
+ * An instance method defined within an `ActionController` controller class.
+ * This may be the target of a route handler, if such a route is defined.
  */
-class ActionControllerParamsCall extends MethodCall {
+class ActionControllerActionMethod extends Method, HTTP::Server::RequestHandler::Range {
   private ActionControllerControllerClass controllerClass;
 
-  ActionControllerParamsCall() {
-    this.getMethodName() = "params" and
+  ActionControllerActionMethod() { this = controllerClass.getAMethod() }
+
+  /**
+   * Establishes a mapping between a method within the file
+   * `<sourcePrefix>app/controllers/<subpath>_controller.rb` and the
+   * corresponding template file at
+   * `<sourcePrefix>app/views/<subpath>/<method_name>.html.erb`.
+   */
+  // TODO: result should be `ErbFile`
+  File getDefaultTemplateFile() {
+    controllerTemplatesFolder(this.getControllerClass(), result.getParentContainer()) and
+    result.getBaseName() = this.getName() + ".html.erb"
+  }
+
+  // params come from `params` method rather than a method parameter
+  override Parameter getARoutedParameter() { none() }
+
+  override string getFramework() { result = "ActionController" }
+
+  /** Gets a call to render from within this method. */
+  RenderCall getARenderCall() { result.getParent+() = this }
+
+  // TODO: model the implicit render call when a path through the method does
+  // not end at an explicit render or redirect
+  /** Gets the controller class containing this method. */
+  ActionControllerControllerClass getControllerClass() { result = controllerClass }
+}
+
+// A method call with a `self` receiver from within a controller class
+private class ActionControllerContextCall extends MethodCall {
+  private ActionControllerControllerClass controllerClass;
+
+  ActionControllerContextCall() {
     this.getReceiver() instanceof Self and
     this.getEnclosingModule() = controllerClass
   }
@@ -73,14 +101,120 @@ class ActionControllerParamsCall extends MethodCall {
 }
 
 /**
- * A `RemoteFlowSource::Range` to represent accessing the Action Controller
- * parameters available to a controller via the `params` method.
+ * A call to the `params` method to fetch the request parameters.
  */
-class ActionControllerParamsSource extends RemoteFlowSource::Range {
-  ActionControllerParamsCall call;
+abstract class ParamsCall extends MethodCall {
+  ParamsCall() { this.getMethodName() = "params" }
+}
 
-  ActionControllerParamsSource() { this.asExpr().getExpr() = call }
+/**
+ * A `RemoteFlowSource::Range` to represent accessing the
+ * ActionController parameters available via the `params` method.
+ */
+class ParamsSource extends RemoteFlowSource::Range {
+  ParamsCall call;
+
+  ParamsSource() { this.asExpr().getExpr() = call }
 
-  // TODO: what to use here?
   override string getSourceType() { result = "ActionController::Metal#params" }
 }
+
+// A call to `params` from within a controller.
+private class ActionControllerParamsCall extends ActionControllerContextCall, ParamsCall { }
+
+// A call to `render` from within a controller.
+private class ActionControllerRenderCall extends ActionControllerContextCall, RenderCall { }
+
+// A call to `render_to` from within a controller.
+private class ActionControllerRenderToCall extends ActionControllerContextCall, RenderToCall { }
+
+// A call to `html_safe` from within a controller.
+private class ActionControllerHtmlSafeCall extends HtmlSafeCall {
+  ActionControllerHtmlSafeCall() {
+    this.getEnclosingModule() instanceof ActionControllerControllerClass
+  }
+}
+
+/**
+ * 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 {
+  RedirectToCall() { this.getMethodName() = "redirect_to" }
+
+  /** Gets the `Expr` representing the URL to redirect to, if any */
+  Expr getRedirectUrl() { result = this.getArgument(0) }
+
+  /** Gets the `ActionControllerActionMethod` to redirect to, if any */
+  ActionControllerActionMethod getRedirectActionMethod() {
+    exists(string methodName |
+      methodName = this.getKeywordArgument("action").(StringlikeLiteral).getValueText() and
+      methodName = result.getName() and
+      result.getEnclosingModule() = this.getControllerClass()
+    )
+  }
+}
+
+/**
+ * A method in an `ActionController` class that is accessible from within a
+ * Rails view as a helper method. For instance, in:
+ *
+ * ```rb
+ * class FooController < ActionController::Base
+ *   helper_method :logged_in?
+ *   def logged_in?
+ *     @current_user != nil
+ *   end
+ * end
+ * ```
+ *
+ * the `logged_in?` method is a helper method.
+ * See also https://api.rubyonrails.org/classes/AbstractController/Helpers/ClassMethods.html#method-i-helper_method
+ */
+class ActionControllerHelperMethod extends Method {
+  private ActionControllerControllerClass controllerClass;
+
+  ActionControllerHelperMethod() {
+    this.getEnclosingModule() = controllerClass and
+    exists(MethodCall helperMethodMarker |
+      helperMethodMarker.getMethodName() = "helper_method" and
+      helperMethodMarker.getAnArgument().(StringlikeLiteral).getValueText() = this.getName() and
+      helperMethodMarker.getEnclosingModule() = controllerClass
+    )
+  }
+
+  /** Gets the class containing this helper method. */
+  ActionControllerControllerClass getControllerClass() { result = controllerClass }
+}
+
+/**
+ * Gets an `ActionControllerControllerClass` 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.
+ */
+// TODO: parameter should be `ErbFile`
+ActionControllerControllerClass getAssociatedControllerClass(File f) {
+  controllerTemplatesFolder(result, f.getParentContainer())
+}
+
+/**
+ * Holds if `templatesFolder` is in the correct location to contain template
+ * files "belonging" to the given `ActionControllerControllerClass`, according
+ * to Rails conventions.
+ *
+ * In particular, this means that an action method in `cls` will by default
+ * render a correspondingly named template file within `templatesFolder`.
+ */
+predicate controllerTemplatesFolder(ActionControllerControllerClass cls, Folder templatesFolder) {
+  exists(string templatesPath, string sourcePrefix, string subPath, string controllerPath |
+    controllerPath = cls.getLocation().getFile().getRelativePath() and
+    templatesPath = templatesFolder.getRelativePath() and
+    // `sourcePrefix` is either a prefix path ending in a slash, or empty if
+    // the rails app is at the source root
+    sourcePrefix = [controllerPath.regexpCapture("^(.*/)app/controllers/(?:.*?)/(?:[^/]*)$", 1), ""] and
+    controllerPath = sourcePrefix + "app/controllers/" + subPath + "_controller.rb" and
+    templatesPath = sourcePrefix + "app/views/" + subPath
+  )
+}

ql/src/codeql_ruby/frameworks/ActionView.qll

@@ -0,0 +1,133 @@
+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 import ActionController
+
+predicate inActionViewContext(AstNode n) {
+  // Within a template
+  // TODO: n.getLocation().getFile() instanceof ErbFile
+  n.getLocation().getFile().getExtension() = "erb"
+}
+
+/**
+ * A method call on a string to mark it as HTML safe for Rails.
+ * Strings marked as such will not be automatically escaped when inserted into
+ * HTML.
+ */
+abstract class HtmlSafeCall extends MethodCall {
+  HtmlSafeCall() { this.getMethodName() = "html_safe" }
+}
+
+// A call to `html_safe` from within a template or view component.
+private class ActionViewHtmlSafeCall extends HtmlSafeCall {
+  ActionViewHtmlSafeCall() { inActionViewContext(this) }
+}
+
+// A call in a context where some commonly used `ActionView` methods are available.
+private class ActionViewContextCall extends MethodCall {
+  ActionViewContextCall() {
+    this.getReceiver() instanceof Self and
+    inActionViewContext(this)
+  }
+
+  predicate isInErbFile() {
+    // TODO: this.getLocation().getFile() instanceof ErbFile
+    this.getLocation().getFile().getExtension() = "erb"
+  }
+}
+
+/** A call to the `raw` method to output a value without HTML escaping. */
+class RawCall extends ActionViewContextCall {
+  RawCall() { this.getMethodName() = "raw" }
+}
+
+// A call to the `params` method within the context of a template or view component.
+private class ActionViewParamsCall extends ActionViewContextCall, ParamsCall { }
+
+/**
+ * A call to a `render` method that will populate the response body with the
+ * rendered content.
+ */
+abstract class RenderCall extends MethodCall {
+  RenderCall() { this.getMethodName() = "render" }
+
+  private string getWorkingDirectory() {
+    result = this.getLocation().getFile().getParentContainer().getAbsolutePath()
+  }
+
+  bindingset[templatePath]
+  private string templatePathPattern(string templatePath) {
+    exists(string basename, string relativeRoot |
+      // everything after the final slash, or the whole string if there is no slash
+      basename = templatePath.regexpCapture("^(?:.*/)?([^/]*)$", 1) and
+      // everything up to and including the final slash
+      relativeRoot = templatePath.regexpCapture("^(.*/)?(?:[^/]*?)$", 1)
+    |
+      (
+        // path relative to <source_prefix>/app/views/
+        result = "%/app/views/" + relativeRoot + "%" + basename + "%"
+        or
+        // relative to file containing call
+        result = this.getWorkingDirectory() + "%" + templatePath + "%"
+      )
+    )
+  }
+
+  private string getTemplatePathPatterns() {
+    exists(string templatePath |
+      exists(Expr arg |
+        // TODO: support other ways of specifying paths (e.g. `file`)
+        arg = this.getKeywordArgument("partial") or
+        arg = this.getKeywordArgument("template") or
+        arg = this.getKeywordArgument("action") or
+        arg = this.getArgument(0)
+      |
+        templatePath = arg.(StringlikeLiteral).getValueText()
+      )
+    |
+      result = this.templatePathPattern(templatePath)
+    )
+  }
+
+  /**
+   * Get the template file to be rendered by this call, if any.
+   */
+  // TODO: parameter should be `ErbFile`
+  File getTemplateFile() { result.getAbsolutePath().matches(this.getTemplatePathPatterns()) }
+
+  /**
+   * Get the local variables passed as context to the renderer
+   */
+  HashLiteral getLocals() { result = this.getKeywordArgument("locals") }
+  // TODO: implicit renders in controller actions
+}
+
+// A call to the `render` method within the context of a template or view component.
+private class ActionViewRenderCall extends RenderCall, ActionViewContextCall { }
+
+/**
+ * A render call that does not automatically set the HTTP response body.
+ */
+abstract class RenderToCall extends MethodCall {
+  RenderToCall() { this.getMethodName() = ["render_to_body", "render_to_string"] }
+}
+
+// A call to `render_to` from within a template or view component.
+private class ActionViewRenderToCall extends ActionViewContextCall, RenderToCall { }
+
+/**
+ * A call to the ActionView `link_to` helper method.
+ *
+ * This generates an HTML anchor tag. The method is not designed to expect
+ * user-input, so provided paths are not automatically HTML escaped.
+ */
+class LinkToCall extends ActionViewContextCall {
+  LinkToCall() { this.getMethodName() = "link_to" }
+
+  // TODO: the path can also be specified through other optional arguments
+  Expr getPathArgument() { result = this.getArgument(1) }
+}
+// TODO: model flow in/out of template files properly,

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

@@ -2,7 +2,16 @@ actionControllerControllerClasses
 | ActiveRecordInjection.rb:27:1:58:3 | FooController |
 | ActiveRecordInjection.rb:60:1:90:3 | BarController |
 | ActiveRecordInjection.rb:92:1:96:3 | BazController |
-actionControllerParamsCalls
+| app/controllers/foo/bars_controller.rb:1:1:20:3 | BarsController |
+actionControllerActionMethods
+| ActiveRecordInjection.rb:32:3:57:5 | some_request_handler |
+| ActiveRecordInjection.rb:61:3:69:5 | some_other_request_handler |
+| ActiveRecordInjection.rb:71:3:89:5 | safe_paths |
+| ActiveRecordInjection.rb:93:3:95:5 | yet_another_handler |
+| app/controllers/foo/bars_controller.rb:3:3:5:5 | index |
+| app/controllers/foo/bars_controller.rb:7:3:13:5 | show_debug |
+| app/controllers/foo/bars_controller.rb:15:3:19:5 | show |
+paramsCalls
 | ActiveRecordInjection.rb:35:30:35:35 | call to params |
 | ActiveRecordInjection.rb:39:30:39:35 | call to params |
 | ActiveRecordInjection.rb:43:32:43:37 | call to params |
@@ -16,7 +25,12 @@ actionControllerParamsCalls
 | ActiveRecordInjection.rb:83:12:83:17 | call to params |
 | ActiveRecordInjection.rb:88:15:88:20 | call to params |
 | ActiveRecordInjection.rb:94:22:94:27 | call to params |
-actionControllerParamsSources
+| app/controllers/foo/bars_controller.rb:8:21:8:26 | call to params |
+| app/controllers/foo/bars_controller.rb:9:10:9:15 | call to params |
+| app/controllers/foo/bars_controller.rb:16:21:16:26 | call to params |
+| app/controllers/foo/bars_controller.rb:17:10:17:15 | call to params |
+| app/views/foo/bars/show.html.erb:5:9:5:14 | call to params |
+paramsSources
 | ActiveRecordInjection.rb:35:30:35:35 | call to params |
 | ActiveRecordInjection.rb:39:30:39:35 | call to params |
 | ActiveRecordInjection.rb:43:32:43:37 | call to params |
@@ -30,3 +44,16 @@ actionControllerParamsSources
 | ActiveRecordInjection.rb:83:12:83:17 | call to params |
 | ActiveRecordInjection.rb:88:15:88:20 | call to params |
 | ActiveRecordInjection.rb:94:22:94:27 | call to params |
+| app/controllers/foo/bars_controller.rb:8:21:8:26 | call to params |
+| app/controllers/foo/bars_controller.rb:9:10:9:15 | call to params |
+| app/controllers/foo/bars_controller.rb:16:21:16:26 | call to params |
+| app/controllers/foo/bars_controller.rb:17:10:17:15 | call to params |
+| app/views/foo/bars/show.html.erb:5:9:5:14 | call to params |
+redirectToCalls
+| app/controllers/foo/bars_controller.rb:12:5:12:30 | call to redirect_to |
+actionControllerHelperMethods
+getAssociatedControllerClasses
+| app/controllers/foo/bars_controller.rb:1:1:20:3 | BarsController | app/views/foo/bars/_widget.html.erb:0:0:0:0 | app/views/foo/bars/_widget.html.erb |
+| app/controllers/foo/bars_controller.rb:1:1:20:3 | BarsController | app/views/foo/bars/show.html.erb:0:0:0:0 | app/views/foo/bars/show.html.erb |
+controllerTemplatesFolders
+| app/controllers/foo/bars_controller.rb:1:1:20:3 | BarsController | folder://app/views/foo/bars | app/views/foo/bars |

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

@@ -1,8 +1,24 @@
-import codeql_ruby.controlflow.CfgNodes
-import codeql_ruby.frameworks.ActionController
+private import ruby
+private import codeql_ruby.frameworks.ActionController
+private import codeql_ruby.frameworks.ActionView
 
 query predicate actionControllerControllerClasses(ActionControllerControllerClass cls) { any() }
 
-query predicate actionControllerParamsCalls(ActionControllerParamsCall call) { any() }
+query predicate actionControllerActionMethods(ActionControllerActionMethod m) { any() }
 
-query predicate actionControllerParamsSources(ActionControllerParamsSource source) { any() }
+query predicate paramsCalls(ParamsCall c) { any() }
+
+query predicate paramsSources(ParamsSource src) { any() }
+
+query predicate redirectToCalls(RedirectToCall c) { any() }
+
+query predicate actionControllerHelperMethods(ActionControllerHelperMethod m) { any() }
+
+// TODO: second parameter should be `ErbFile`
+query predicate getAssociatedControllerClasses(ActionControllerControllerClass cls, File f) {
+  cls = getAssociatedControllerClass(f)
+}
+
+query predicate controllerTemplatesFolders(ActionControllerControllerClass cls, Folder f) {
+  controllerTemplatesFolder(cls, f)
+}

ql/test/library-tests/frameworks/ActionView.expected

@@ -0,0 +1,21 @@
+htmlSafeCalls
+| app/views/foo/bars/show.html.erb:23:3:23:25 | call to html_safe |
+| app/views/foo/bars/show.html.erb:27:3:27:25 | call to html_safe |
+rawCalls
+| app/views/foo/bars/_widget.html.erb:1:5:1:21 | call to raw |
+| app/views/foo/bars/_widget.html.erb:2:5:2:20 | call to raw |
+| app/views/foo/bars/_widget.html.erb:3:5:3:29 | call to raw |
+| app/views/foo/bars/show.html.erb:1:14:1:29 | call to raw |
+| app/views/foo/bars/show.html.erb:2:5:2:21 | call to raw |
+| app/views/foo/bars/show.html.erb:3:5:3:20 | call to raw |
+| app/views/foo/bars/show.html.erb:4:5:4:29 | call to raw |
+| app/views/foo/bars/show.html.erb:5:5:5:21 | call to raw |
+| app/views/foo/bars/show.html.erb:7:5:7:19 | call to raw |
+renderCalls
+| app/controllers/foo/bars_controller.rb:4:5:4:37 | call to render |
+| app/controllers/foo/bars_controller.rb:18:5:18:76 | call to render |
+| app/views/foo/bars/show.html.erb:31:5:31:89 | call to render |
+renderToCalls
+| app/controllers/foo/bars_controller.rb:10:16:10:97 | call to render_to_string |
+linkToCalls
+| app/views/foo/bars/show.html.erb:33:5:33:41 | call to link_to |

ql/test/library-tests/frameworks/ActionView.ql

@@ -0,0 +1,12 @@
+import codeql_ruby.frameworks.ActionController
+import codeql_ruby.frameworks.ActionView
+
+query predicate htmlSafeCalls(HtmlSafeCall c) { any() }
+
+query predicate rawCalls(RawCall c) { any() }
+
+query predicate renderCalls(RenderCall c) { any() }
+
+query predicate renderToCalls(RenderToCall c) { any() }
+
+query predicate linkToCalls(LinkToCall c) { any() }

ql/test/library-tests/frameworks/app/components/DummyComponent.rb

@@ -0,0 +1,2 @@
+class DummyComponent < ViewComponent::Base
+end

ql/test/library-tests/frameworks/app/controllers/foo/bars_controller.rb

@@ -0,0 +1,20 @@
+class BarsController < ApplicationController
+
+  def index
+    render template: "foo/bars/index"
+  end
+
+  def show_debug
+    @user_website = params[:website]
+    dt = params[:text]
+    rendered = render_to_string "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
+    puts rendered
+    redirect_to action: "show"
+  end
+
+  def show
+    @user_website = params[:website]
+    dt = params[:text]
+    render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
+  end
+end

ql/test/library-tests/frameworks/app/views/foo/bars/_widget.html.erb

@@ -0,0 +1,4 @@
+<%= raw @display_text %>
+<%= raw display_text %>
+<%= raw locals[:display_text] %>
+<%= @display_text %>

ql/test/library-tests/frameworks/app/views/foo/bars/show.html.erb

@@ -0,0 +1,33 @@
+<a href="<%= raw user_website %>">website</a>
+<%= raw @display_text %>
+<%= raw display_text %>
+<%= raw locals[:display_text] %>
+<%= raw params[:text] %>
+<% key = :display_text %>
+<%= raw locals[key] %>
+
+<ul>
+<% key = [:display_text, :safe_text] do
+  <li><%= raw locals[key] %></li>
+<% end %>
+</ul>
+
+<%= @display_text %>
+
+<%=
+  full_text = prefix + locals[:display_text]
+  full_text
+%>
+
+<%=
+  @display_text.html_safe
+%>
+
+<%=
+  @display_text.html_safe
+  @display_text
+%>
+
+<%= render partial: 'foo/bars/widget', locals: { display_text: "widget_" + display_text } %>
+
+<%= link_to "some website", @user_website %>