Merge pull request #133 from presidentbeef/show_render_chain_for_templates

Track render chain for templates

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -137,7 +137,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
   #Otherwise, ignore
   def process_escaped_output exp
     unless check_for_immediate_xss exp
-      if raw_call? exp
+      if raw_call? exp and not duplicate? exp
         process exp.value.first_arg
       end
     end

lib/brakeman/format/style.css

@@ -116,3 +116,18 @@ p {
  .user_input {
    background-color: #fcecab;
  }
+
+ div.render_path {
+   display: none;
+   background-color: #ffe;
+   padding: 5px;
+   margin: 2px 0px 2px 0px;
+ }
+
+ div.template_name {
+   cursor: pointer;
+ }
+
+ div.template_name:hover {
+   background-color: white;
+ }

lib/brakeman/processor.rb

@@ -72,7 +72,7 @@ module Brakeman
       #Each template which is rendered is stored separately
       #with a new name.
       if called_from
-        name = (name.to_s + "." + called_from.to_s).to_sym
+        name = ("#{name}.#{called_from}").to_sym
       end
 
       @tracker.templates[name][:src] = result

lib/brakeman/processors/controller_alias_processor.rb

@@ -168,7 +168,7 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor
 
   #Process template and add the current class and method name as called_from info
   def process_template name, args
-    super name, args, "#@current_class##@current_method"
+    super name, args, ["#@current_class##@current_method"]
   end
 
   #Turns a method name into a template name

lib/brakeman/processors/template_alias_processor.rb

@@ -17,12 +17,16 @@ class Brakeman::TemplateAliasProcessor < Brakeman::AliasProcessor
 
   #Process template
   def process_template name, args
-    if @called_from and @called_from.match(/Template:#{name}$/)
-      Brakeman.debug "Skipping circular render from #{@template[:name]} to #{name}"
-      return
-    end
+    if @called_from
+      unless @called_from.grep(/Template:#{name}$/).empty?
+        Brakeman.debug "Skipping circular render from #{@template[:name]} to #{name}"
+        return
+      end
 
-    super name, args, "Template:#{@template[:name]}"
+      super name, args, @called_from + ["Template:#{@template[:name]}"]
+    else
+      super name, args, ["Template:#{@template[:name]}"]
+    end
   end
 
   #Determine template name

lib/brakeman/report.rb

@@ -136,6 +136,8 @@ class Brakeman::Report
           w["Confidence"] = HTML_CONFIDENCE[w["Confidence"]]
           w["Message"] = with_context warning, w["Message"]
           w["Warning Type"] = with_link warning, w["Warning Type"]
+          w["Called From"] = warning.called_from
+          w["Template Name"] = warning.template[:name]
         else
           w["Confidence"] = TEXT_CONFIDENCE[w["Confidence"]]
           w["Message"] = text_message warning, w["Message"]

lib/brakeman/templates/view_warnings.html.erb

@@ -6,12 +6,25 @@
     <th>Warning Type</th>
     <th>Message</th>
   </tr>
-<% warnings.each do |warning| %>
+<% warnings.each_with_index do |warning, i| %>
   <tr>
     <td><%= warning['Confidence']%></td>
-    <td><%= warning['Template']%></td>
+    <td>
+      <% if warning['Called From'] and warning['Called From'].length > 1 %>
+        <div class="template_name" onClick="toggle('callers<%= i %>')" >
+          <div>
+            <%= warning['Template'] %>
+          </div>
+          <div class="render_path" id="callers<%= i %>" >
+            <%= warning['Called From'].join(' &rarr; ') %> &rarr; <%= warning['Template Name'] %>
+          </div>
+        </div>
+      <% else %>
+        <%= warning['Template']%>
+      <% end %>
+    </td>
     <td><%= warning['Warning Type']%></td>
     <td><%= warning['Message']%></td>
   </tr>
 <% end %>
-</table>
\ No newline at end of file
+</table>

lib/brakeman/warning.rb

@@ -66,7 +66,7 @@ class Brakeman::Warning
   def view_name
     return @view_name if @view_name
     if called_from
-      @view_name = "#{template[:name]} (#{called_from})"
+      @view_name = "#{template[:name]} (#{called_from.last})"
     else
       @view_name = template[:name]
     end

test/apps/rails3.1/app/controllers/other_controller.rb

@@ -26,4 +26,14 @@ class OtherController < ApplicationController
   def g
     @g = params[:bad]
   end
+
+  def test_partial1
+    @a = params[:bad!]
+    render :test_partial
+  end
+
+  def test_partial2
+    @b = params[:badder!]
+    render :test_partial
+  end
 end

test/apps/rails3.1/app/views/other/_partial.html.erb

+<%= raw @a %>
+
+<%= raw @b %>

test/apps/rails3.1/app/views/other/test_partial.html.erb

+<%= render 'partial' %>

test/apps/rails3.1/config/routes.rb

@@ -13,6 +13,8 @@ Rails31::Application.routes.draw do
     get 'b'
     post 'something' => 'c'
     put 'dee', :to => :d
+    get 'test_partial1'
+    get 'test_partial2'
   end
 
   match 'e', :to => 'other#e', :as => 'eeeee'

test/tests/test_rails31.rb

@@ -13,7 +13,7 @@ class Rails31Tests < Test::Unit::TestCase
   def expected
     @expected ||= {
       :model => 0,
-      :template => 12,
+      :template => 14,
       :controller => 1,
       :warning => 48 }
   end