diff --git a/lib/brakeman/checks/check_cross_site_scripting.rb b/lib/brakeman/checks/check_cross_site_scripting.rb index 1219b25252179d9ec040679b34f06c67bd0cc0f4..204f553e4c2205b991ebb88c2ecb81fb77005010 100644 --- a/lib/brakeman/checks/check_cross_site_scripting.rb +++ b/lib/brakeman/checks/check_cross_site_scripting.rb @@ -57,8 +57,12 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck if exp.node_type == :output out = exp.value - elsif exp.node_type == :escaped_output and raw_call? exp - out = exp.value.first_arg + elsif exp.node_type == :escaped_output + if raw_call? exp + out = exp.value.first_arg + elsif html_safe_call? exp + out = exp.value.target + end end return if call? out and ignore_call? out.target, out.method @@ -143,8 +147,12 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck #Otherwise, ignore def process_escaped_output exp unless check_for_immediate_xss exp - if raw_call? exp and not duplicate? exp - process exp.value.first_arg + if not duplicate? exp + if raw_call? exp + process exp.value.first_arg + elsif html_safe_call? exp + process exp.value.target + end end end exp @@ -327,6 +335,10 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck exp.value.node_type == :call and exp.value.method == :raw end + def html_safe_call? exp + exp.value.node_type == :call and exp.value.method == :html_safe + end + def ignore_call? target, method ignored_method?(target, method) or safe_input_attribute?(target, method) or diff --git a/test/apps/rails4/app/views/another/html_safe_is_not.html.erb b/test/apps/rails4/app/views/another/html_safe_is_not.html.erb new file mode 100644 index 0000000000000000000000000000000000000000..0047727843220378219b61d6f99bdfa15ce9854b --- /dev/null +++ b/test/apps/rails4/app/views/another/html_safe_is_not.html.erb @@ -0,0 +1 @@ +<%= params[:x].html_safe %> diff --git a/test/tests/rails4.rb b/test/tests/rails4.rb index 1dcade6fefd263cb9eac072e0a685282e58b4d8b..e207b7e1a1fcb3328f250d74fc41e810c85f115e 100644 --- a/test/tests/rails4.rb +++ b/test/tests/rails4.rb @@ -13,7 +13,7 @@ class Rails4Tests < Test::Unit::TestCase @expected ||= { :controller => 0, :model => 2, - :template => 6, + :template => 7, :generic => 61 } end @@ -572,6 +572,18 @@ class Rails4Tests < Test::Unit::TestCase :user_input => nil end + def test_cross_site_scripting_with_html_safe + assert_warning :type => :template, + :warning_code => 2, + :fingerprint => "b04cfd8d120b773a3e9f70af8762f7efa7c5ca5c7f83136131d6cc75259cd429", + :warning_type => "Cross Site Scripting", + :line => 1, + :message => /^Unescaped\ parameter\ value/, + :confidence => 0, + :relative_path => "app/views/another/html_safe_is_not.html.erb", + :user_input => nil + end + def test_xss_haml_line_number assert_warning :type => :template, :warning_code => 2,