fix some local variable mappings between view and controller

Author: alexrford

ql/lib/codeql/ruby/security/ReflectedXSSCustomizations.qll

@@ -1,9 +1,3 @@
-/**
- * Provides default sources, sinks and sanitizers for detecting
- * "reflected server-side cross-site scripting"
- * vulnerabilities, as well as extension points for adding your own.
- */
-
 private import ruby
 private import codeql.ruby.DataFlow
 private import codeql.ruby.CFG
@@ -122,19 +116,16 @@ module ReflectedXSS {
       call.getTemplateFile() = node2.getLocation().getFile() and
       (
         // ...node2 is a variable read
-        exists(CfgNodes::ExprNodes::VariableReadAccessCfgNode readNode |
-          readNode = node2.asExpr() and
-          readNode.getExpr().getVariable().getName() = "@" + hashKey
-        )
+        node2.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = hashKey
         or
-        // ...node2 is an element reference against `locals`
+        // ...node2 is an element reference against `local_assigns`
         exists(
           CfgNodes::ExprNodes::ElementReferenceCfgNode refNode, DataFlow::Node argNode,
           CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode
         |
           refNode = node2.asExpr() and
           argNode.asExpr() = refNode.getArgument(0) and
-          refNode.getReceiver().getExpr().(MethodCall).getMethodName() = "locals" and
+          refNode.getReceiver().getExpr().(MethodCall).getMethodName() = "local_assigns" and
           argNode.getALocalSource() = DataFlow::exprNode(strNode) and
           strNode.getExpr().getValueText() = hashKey
         )

ql/test/query-tests/security/cwe-079/app/views/foo/bars/_widget.html.erb

@@ -4,8 +4,8 @@
 <%# BAD: A local rendered raw as a local variable %>
 <%= raw display_text %>
 
-<%# BAD: A local rendered raw via the locals hash %>
-<%= raw locals[:display_text] %>
+<%# BAD: A local rendered raw via the local_assigns hash %>
+<%= raw local_assigns[:display_text] %>
 
-<%# GOOD: A local rendered with default escaping via the locals hash %>
-<%= @display_text %>
+<%# GOOD: A local rendered with default escaping via the local_assigns hash %>
+<%= local_assigns[:display_text] %>

ql/test/query-tests/security/cwe-079/app/views/foo/bars/show.html.erb

@@ -1,39 +1,36 @@
 <%# BAD: An instance variable rendered without escaping %>
 <a href="<%= raw user_website %>">website</a>
 
-<%# BAD: A local rendered raw as an instance variable %>
-<%= raw @display_text %>
-
 <%# BAD: A local rendered raw as a local variable %>
 <%= raw display_text %>
 
-<%# BAD: A local rendered raw via the locals hash %>
-<%= raw locals[:display_text] %>
+<%# BAD: A local rendered raw via the local_assigns hash %>
+<%= raw local_assigns[:display_text] %>
 
 <% key = :display_text %>
-<%# BAD: A local rendered raw via the locals hash %>
-<%= raw locals[key] %>
+<%# BAD: A local rendered raw via the locals_assigns hash %>
+<%= raw local_assigns[key] %>
 
 <ul>
 <% for key in [:display_text, :safe_text] do %>
   <%# BAD: A local rendered raw via the locals hash %>
   <%# TODO: we miss that `key` can take `:display_text` as a value here %>
-  <li><%= raw locals[key] %></li>
+  <li><%= raw local_assigns[key] %></li>
 <% end %>
 </ul>
 
-<%# GOOD: A local rendered with default escaping via the locals hash %>
-<%= @display_text %>
+<%# GOOD: A local rendered with default escaping via the local_assigns hash %>
+<%= local_assigns[display_text] %>
 
 <%# GOOD: default escaping of rendered text %>
 <%=
-  full_text = prefix + locals[:display_text]
+  full_text = prefix + local_assigns[:display_text]
   full_text
 %>
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>
 <%=
-  @display_text.html_safe
+  display_text.html_safe
 %>
 
 <%# BAD: html_safe marks string as not requiring HTML escaping %>