diff --git a/lib/brakeman/processors/haml_template_processor.rb b/lib/brakeman/processors/haml_template_processor.rb index 400f542c44a77170f576ccfdc42d5de974171bce..9bd31bf902d8c05ef64df32f020d31282ac49ddf 100644 --- a/lib/brakeman/processors/haml_template_processor.rb +++ b/lib/brakeman/processors/haml_template_processor.rb @@ -3,6 +3,7 @@ require 'brakeman/processors/template_processor' #Processes HAML templates. class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor HAML_FORMAT_METHOD = /format_script_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)/ + HAML_HELPERS = s(:colon2, s(:const, :Haml), :Helpers) #Processes call, looking for template output def process_call exp @@ -37,9 +38,7 @@ class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor else case method.to_s when "push_text" - s = Sexp.new(:output, out) - @current_template[:outputs] << s - s + build_output_from_push_text(out) when HAML_FORMAT_METHOD if $4 == "true" Sexp.new :format_escaped, out @@ -117,4 +116,52 @@ class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor exp.target.value == :_hamlout and exp.method == :buffer end + + #HAML likes to put interpolated values into _hamlout.push_text + #but we want to handle those individually + def build_output_from_push_text exp + if node_type? exp, :string_interp, :dstr + exp.map! do |e| + if sexp? e + if node_type? e, :string_eval, :evstr + e = e.value + end + + get_pushed_value e + else + e + end + end + end + end + + #Gets outputs from values interpolated into _hamlout.push_text + def get_pushed_value exp + return exp unless sexp? exp + + case exp.node_type + when :format + exp.node_type = :output + @current_template[:outputs] << exp + exp + when :format_escaped + exp.node_type = :escaped_output + @current_template[:outputs] << exp + exp + when :str, :ignore, :output, :escaped_output + exp + when :block, :rlist, :string_interp, :dstr + exp.map! { |e| get_pushed_value e } + else + if call? exp and exp.target == HAML_HELPERS and exp.method == :html_escape + s = Sexp.new(:escaped_output, exp.first_arg) + else + s = Sexp.new(:output, exp) + end + + s.line(exp.line) + @current_template[:outputs] << s + s + end + end end diff --git a/test/apps/rails2/app/controllers/other_controller.rb b/test/apps/rails2/app/controllers/other_controller.rb index 828f1e5fdf04a14a23c312b7f77958abda143c80..87181ccf01d4d515bf8f410ca3a98f82f394abac 100644 --- a/test/apps/rails2/app/controllers/other_controller.rb +++ b/test/apps/rails2/app/controllers/other_controller.rb @@ -65,4 +65,8 @@ class OtherController < ApplicationController render :xss_dupes, :layout => 'thing' end + + def test_haml_stuff + render :locals => { :user => User.first } + end end diff --git a/test/apps/rails2/app/views/other/test_haml_stuff.html.haml b/test/apps/rails2/app/views/other/test_haml_stuff.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..494f5898919099f4031c1b0df51ca521afed2873 --- /dev/null +++ b/test/apps/rails2/app/views/other/test_haml_stuff.html.haml @@ -0,0 +1,4 @@ +%tr + %td= user.age.to_i + %td= user.stuff + %td= user.status diff --git a/test/tests/rails2.rb b/test/tests/rails2.rb index e7c0a194d6024e10f77852ef715176ffb4fb79aa..f42c9818b0b6cec79a962c012c3279251aef6518 100644 --- a/test/tests/rails2.rb +++ b/test/tests/rails2.rb @@ -11,13 +11,13 @@ class Rails2Tests < Test::Unit::TestCase @expected ||= { :controller => 1, :model => 3, - :template => 45, + :template => 47, :generic => 49 } else @expected ||= { :controller => 1, :model => 3, - :template => 45, + :template => 47, :generic => 50 } end end @@ -1119,6 +1119,30 @@ class Rails2Tests < Test::Unit::TestCase :relative_path => "app/views/layouts/thing.html.erb" end + def test_cross_site_scripting_in_haml + assert_warning :type => :template, + :warning_code => 2, + :fingerprint => "702f9bae476402bb2614794276083849342540bd8b5e8f2fc35b15b40e9f34fc", + :warning_type => "Cross Site Scripting", + :line => 3, + :message => /^Unescaped\ model\ attribute/, + :confidence => 0, + :relative_path => "app/views/other/test_haml_stuff.html.haml", + :user_input => nil + end + + def test_cross_site_scripting_in_haml2 + assert_warning :type => :template, + :warning_code => 2, + :fingerprint => "79cbc87a06ad9247362be97ba4b6cc12b9619fd0f68d468b81cbed376bfbcc5c", + :warning_type => "Cross Site Scripting", + :line => 4, + :message => /^Unescaped\ model\ attribute/, + :confidence => 0, + :relative_path => "app/views/other/test_haml_stuff.html.haml", + :user_input => nil + end + def test_dangerous_send_try assert_warning :type => :warning, :warning_type => "Dangerous Send", @@ -1287,13 +1311,13 @@ class Rails2WithOptionsTests < Test::Unit::TestCase @expected ||= { :controller => 1, :model => 4, - :template => 45, + :template => 47, :generic => 49 } else @expected ||= { :controller => 1, :model => 4, - :template => 45, + :template => 47, :generic => 50 } end end diff --git a/test/tests/tabs_output.rb b/test/tests/tabs_output.rb index 86ed80a4330849f7bdaf755a858ef06bc19465ab..3588f88fa49ab2b0a58221860dfd70862082895e 100644 --- a/test/tests/tabs_output.rb +++ b/test/tests/tabs_output.rb @@ -3,9 +3,9 @@ class TestTabsOutput < Test::Unit::TestCase def test_reported_warnings if Brakeman::Scanner::RUBY_1_9 - assert_equal 99, Report.lines.to_a.count + assert_equal 101, Report.lines.to_a.count else - assert_equal 100, Report.lines.to_a.count + assert_equal 102, Report.lines.to_a.count end end end