Ruby: Model ERB flow for ActionController

Extend the existing modeling for ViewComponent to ActionController.

The `render` method in ActionController behaves in a similar way except
that the ERB file is evaluated in the context of the controller class,
rather than a separate view class. This means we need the value of
`self` at the `render` call to flow to the `self` in the ERB file.

Author: hmac

Diff for: ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -8,6 +8,7 @@ private import codeql.ruby.dataflow.FlowSummary
 private import codeql.ruby.dataflow.SSA
 private import codeql.util.Boolean
 private import codeql.util.Unit
+private import codeql.ruby.controlflow.CfgNodes
 
 /**
  * A `LocalSourceNode` for a `self` variable. This is the implicit `self`
@@ -239,6 +240,7 @@ class NormalCall extends DataFlowCall, TNormalCall {
  */
 private module ViewComponentRenderModeling {
   private import codeql.ruby.frameworks.ViewComponent
+  private import codeql.ruby.frameworks.ActionController
 
   private class RenderMethod extends SummarizedCallable, LibraryCallableToIncludeInTypeTracking {
     RenderMethod() { this = "render view component" }
@@ -250,12 +252,38 @@ private module ViewComponentRenderModeling {
       // use a call-back summary, and adjust it to a method call below
       output = "Argument[0].Parameter[self]" and
       preservesValue = true
+      or
+      input = "Argument[self]" and
+      output = "Argument[self].Parameter[self]" and
+      preservesValue = true
     }
   }
 
   private string invokeToplevelName() { result = "__invoke__toplevel__erb__" }
 
-  /** Holds if `call` should be adjusted to be a method call to `name` on `receiver`. */
+  /**
+   * Holds if `call` should be adjusted to be a method call to `name` on `receiver`.
+   * `call` is the callback call inside the flow summary.
+   * Effectively we generate something like
+   * ```rb
+   * def render(view)
+   *   # ViewComponent
+   *   view()
+   *   # ActionController
+   *   self()
+   * end
+   * ```
+   *
+   * And this adjustment changes it to
+   * ```rb
+   * def render(view)
+   *   # ViewComponent
+   *   view.__invoke__toplevel__erb__()
+   *   # ActionController
+   *   self.__invoke__toplevel__erb__()
+   * end
+   * ```
+   */
   predicate adjustedMethodCall(DataFlowCall call, FlowSummaryNode receiver, string name) {
     exists(RenderMethod render |
       call = TSummaryCall(render, receiver.getSummaryNode()) and
@@ -265,14 +293,29 @@ private module ViewComponentRenderModeling {
 
   /** Holds if `self` belongs to the top-level of an ERB file with matching view class `view`. */
   pragma[nomagic]
-  predicate selfInErbToplevel(SelfVariable self, ViewComponent::ComponentClass view) {
-    self.getDeclaringScope().(Toplevel).getFile() = view.getTemplate()
+  predicate selfInErbToplevel(SelfVariable self, Module view) {
+    self.getDeclaringScope().(Toplevel).getFile() =
+      [
+        view.(ViewComponent::ComponentClass).getTemplate(),
+        view.(ActionControllerClass).getAnAction().getDefaultTemplateFile()
+      ]
   }
 
   Toplevel lookupMethod(ViewComponent::ComponentClass m, string name) {
     result.getFile() = m.getTemplate() and
     name = invokeToplevelName()
   }
+
+  Toplevel lookupMethodCall(DataFlowCall c, Module mod, DataFlow::Node receiver, string name) {
+    name = invokeToplevelName() and
+    exists(ActionControllerActionMethod m, Call summaryCall |
+      m.getControllerClass() = mod and
+      result.getFile() = m.getDefaultTemplateFile() and
+      c.getEnclosingCallable().asLibraryCallable().getACallSimple() = summaryCall and
+      summaryCall.getEnclosingCallable() = m and
+      adjustedMethodCall(c, receiver, name)
+    )
+  }
 }
 
 /** A call for which we want to compute call targets. */
@@ -801,7 +844,11 @@ private CfgScope lookupInstanceMethodCall(DataFlowCall call, string method, bool
   exists(Module tp, DataFlow::Node receiver |
     methodCall(call, pragma[only_bind_into](receiver), pragma[only_bind_into](method)) and
     receiver = trackInstance(tp, exact) and
-    result = lookupMethod(tp, pragma[only_bind_into](method), exact)
+    (
+      result = lookupMethod(tp, pragma[only_bind_into](method), exact)
+      or
+      result = ViewComponentRenderModeling::lookupMethodCall(call, tp, receiver, method)
+    )
   )
 }
 

Diff for: ruby/ql/test/library-tests/dataflow/erb/app/controllers/users_controller.rb

@@ -0,0 +1,16 @@
+class UsersController < ActionController::Base
+    def index
+        @x = source("index")
+        render
+    end
+
+    def show
+        @x = source("show")
+        # implicit call to `render`
+    end
+
+    def edit
+        @x = source("edit")
+        render
+    end
+end

Diff for: ruby/ql/test/library-tests/dataflow/erb/app/views/users/edit.html.erb

@@ -0,0 +1,3 @@
+<%=
+  sink(@x) # $hasValueFlow=edit
+%>

Diff for: ruby/ql/test/library-tests/dataflow/erb/app/views/users/index.html.erb

@@ -0,0 +1,3 @@
+<%=
+  sink @x # $hasValueFlow=index
+%>

Diff for: ruby/ql/test/library-tests/dataflow/erb/app/views/users/show.html.erb

@@ -0,0 +1,3 @@
+<%=
+  sink @x # $ MISSING: hasValueFlow=show
+%>

Diff for: ruby/ql/test/library-tests/dataflow/erb/erb.expected

@@ -1,5 +1,19 @@
 testFailures
+| app/views/users/edit.html.erb:2:8:2:9 | @x | Unexpected result: hasValueFlow=index |
+| app/views/users/index.html.erb:2:8:2:9 | @x | Unexpected result: hasValueFlow=edit |
 edges
+| app/controllers/users_controller.rb:3:9:3:10 | [post] self [@x] | app/controllers/users_controller.rb:4:9:4:14 | self [@x] | provenance |  |
+| app/controllers/users_controller.rb:3:14:3:28 | call to source | app/controllers/users_controller.rb:3:9:3:10 | [post] self [@x] | provenance |  |
+| app/controllers/users_controller.rb:4:9:4:14 | self [@x] | app/views/users/edit.html.erb:2:3:3:1 | self in edit.html.erb [@x] | provenance |  |
+| app/controllers/users_controller.rb:4:9:4:14 | self [@x] | app/views/users/index.html.erb:2:3:3:1 | self in index.html.erb [@x] | provenance |  |
+| app/controllers/users_controller.rb:13:9:13:10 | [post] self [@x] | app/controllers/users_controller.rb:14:9:14:14 | self [@x] | provenance |  |
+| app/controllers/users_controller.rb:13:14:13:27 | call to source | app/controllers/users_controller.rb:13:9:13:10 | [post] self [@x] | provenance |  |
+| app/controllers/users_controller.rb:14:9:14:14 | self [@x] | app/views/users/edit.html.erb:2:3:3:1 | self in edit.html.erb [@x] | provenance |  |
+| app/controllers/users_controller.rb:14:9:14:14 | self [@x] | app/views/users/index.html.erb:2:3:3:1 | self in index.html.erb [@x] | provenance |  |
+| app/views/users/edit.html.erb:2:3:3:1 | self in edit.html.erb [@x] | app/views/users/edit.html.erb:2:8:2:9 | self [@x] | provenance |  |
+| app/views/users/edit.html.erb:2:8:2:9 | self [@x] | app/views/users/edit.html.erb:2:8:2:9 | @x | provenance |  |
+| app/views/users/index.html.erb:2:3:3:1 | self in index.html.erb [@x] | app/views/users/index.html.erb:2:8:2:9 | self [@x] | provenance |  |
+| app/views/users/index.html.erb:2:8:2:9 | self [@x] | app/views/users/index.html.erb:2:8:2:9 | @x | provenance |  |
 | main.rb:3:9:3:9 | x | main.rb:4:26:4:26 | x | provenance |  |
 | main.rb:3:13:3:21 | call to source | main.rb:3:9:3:9 | x | provenance |  |
 | main.rb:4:9:4:12 | view [@x] | main.rb:5:16:5:19 | view [@x] | provenance |  |
@@ -45,6 +59,18 @@ edges
 | view3.rb:6:5:8:7 | self in get [@x] | view3.rb:7:9:7:10 | self [@x] | provenance |  |
 | view3.rb:7:9:7:10 | self [@x] | view3.rb:7:9:7:10 | @x | provenance |  |
 nodes
+| app/controllers/users_controller.rb:3:9:3:10 | [post] self [@x] | semmle.label | [post] self [@x] |
+| app/controllers/users_controller.rb:3:14:3:28 | call to source | semmle.label | call to source |
+| app/controllers/users_controller.rb:4:9:4:14 | self [@x] | semmle.label | self [@x] |
+| app/controllers/users_controller.rb:13:9:13:10 | [post] self [@x] | semmle.label | [post] self [@x] |
+| app/controllers/users_controller.rb:13:14:13:27 | call to source | semmle.label | call to source |
+| app/controllers/users_controller.rb:14:9:14:14 | self [@x] | semmle.label | self [@x] |
+| app/views/users/edit.html.erb:2:3:3:1 | self in edit.html.erb [@x] | semmle.label | self in edit.html.erb [@x] |
+| app/views/users/edit.html.erb:2:8:2:9 | @x | semmle.label | @x |
+| app/views/users/edit.html.erb:2:8:2:9 | self [@x] | semmle.label | self [@x] |
+| app/views/users/index.html.erb:2:3:3:1 | self in index.html.erb [@x] | semmle.label | self in index.html.erb [@x] |
+| app/views/users/index.html.erb:2:8:2:9 | @x | semmle.label | @x |
+| app/views/users/index.html.erb:2:8:2:9 | self [@x] | semmle.label | self [@x] |
 | main.rb:3:9:3:9 | x | semmle.label | x |
 | main.rb:3:13:3:21 | call to source | semmle.label | call to source |
 | main.rb:4:9:4:12 | view [@x] | semmle.label | view [@x] |
@@ -98,6 +124,10 @@ subpaths
 | view2.html.erb:3:5:3:13 | call to source | view2.rb:6:13:6:13 | x | view2.rb:7:9:7:10 | [post] self [@x] | view2.html.erb:3:1:3:14 | [post] self [@x] |
 | view3.html.erb:3:6:3:8 | self [@x] | view3.rb:6:5:8:7 | self in get [@x] | view3.rb:7:9:7:10 | @x | view3.html.erb:3:6:3:8 | call to get |
 #select
+| app/views/users/edit.html.erb:2:8:2:9 | @x | app/controllers/users_controller.rb:3:14:3:28 | call to source | app/views/users/edit.html.erb:2:8:2:9 | @x | $@ | app/controllers/users_controller.rb:3:14:3:28 | call to source | call to source |
+| app/views/users/edit.html.erb:2:8:2:9 | @x | app/controllers/users_controller.rb:13:14:13:27 | call to source | app/views/users/edit.html.erb:2:8:2:9 | @x | $@ | app/controllers/users_controller.rb:13:14:13:27 | call to source | call to source |
+| app/views/users/index.html.erb:2:8:2:9 | @x | app/controllers/users_controller.rb:3:14:3:28 | call to source | app/views/users/index.html.erb:2:8:2:9 | @x | $@ | app/controllers/users_controller.rb:3:14:3:28 | call to source | call to source |
+| app/views/users/index.html.erb:2:8:2:9 | @x | app/controllers/users_controller.rb:13:14:13:27 | call to source | app/views/users/index.html.erb:2:8:2:9 | @x | $@ | app/controllers/users_controller.rb:13:14:13:27 | call to source | call to source |
 | view1.rb:10:14:10:15 | @x | main.rb:3:13:3:21 | call to source | view1.rb:10:14:10:15 | @x | $@ | main.rb:3:13:3:21 | call to source | call to source |
 | view1.rb:10:14:10:15 | @x | view1.html.erb:6:5:6:13 | call to source | view1.rb:10:14:10:15 | @x | $@ | view1.html.erb:6:5:6:13 | call to source | call to source |
 | view2.rb:3:14:3:15 | @x | view2.html.erb:3:5:3:13 | call to source | view2.rb:3:14:3:15 | @x | $@ | view2.html.erb:3:5:3:13 | call to source | call to source |