diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index b158ad1df2444a801f726e1e36e6cd243f6ae137..e916afbb2d2dd60b735574b98a4378ebf0086410 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -516,4 +516,23 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor @active_record_models end + + def friendly_type_of input_type + if input_type.is_a? Match + input_type = input_type.type + end + + case input_type + when :params + "parameter value" + when :cookies + "cookie value" + when :request + "request value" + when :model + "model attribute" + else + "user input" + end + end end diff --git a/lib/brakeman/checks/check_content_tag.rb b/lib/brakeman/checks/check_content_tag.rb index 98713f7df7bb61ea8e150ce9ee09b43d1751ed87..5974fc2c1974b5fd17c9e00c82426c09b0d8850b 100644 --- a/lib/brakeman/checks/check_content_tag.rb +++ b/lib/brakeman/checks/check_content_tag.rb @@ -45,7 +45,7 @@ class Brakeman::CheckContentTag < Brakeman::CheckCrossSiteScripting call = result[:call] = result[:call].dup - args = call.arglist + args = call.arglist tag_name = args[1] content = args[2] @@ -94,19 +94,12 @@ class Brakeman::CheckContentTag < Brakeman::CheckCrossSiteScripting end if input = has_immediate_user_input?(arg) - case input.type - when :params - message = "Unescaped parameter value in content_tag" - when :cookies - message = "Unescaped cookie value in content_tag" - else - message = "Unescaped user input value in content_tag" - end + message = "Unescaped #{friendly_type_of input} in content_tag" add_result result warn :result => result, - :warning_type => "Cross Site Scripting", + :warning_type => "Cross Site Scripting", :warning_code => :xss_content_tag, :message => message, :user_input => input.match, @@ -126,7 +119,7 @@ class Brakeman::CheckContentTag < Brakeman::CheckCrossSiteScripting end warn :result => result, - :warning_type => "Cross Site Scripting", + :warning_type => "Cross Site Scripting", :warning_code => :xss_content_tag, :message => "Unescaped model attribute in content_tag", :user_input => match, @@ -135,28 +128,14 @@ class Brakeman::CheckContentTag < Brakeman::CheckCrossSiteScripting end elsif @matched - message = "Unescaped " - - case @matched.type - when :model - return if tracker.options[:ignore_model_output] - message << "model attribute" - when :params - message << "parameter" - when :cookies - message << "cookie" - when :session - message << "session" - else - message << "user input" - end + return if @matched.type == :model and tracker.options[:ignore_model_output] - message << " value in content_tag" + message = "Unescaped #{friendly_type_of @matched} in content_tag" add_result result - warn :result => result, - :warning_type => "Cross Site Scripting", + warn :result => result, + :warning_type => "Cross Site Scripting", :warning_code => :xss_content_tag, :message => message, :user_input => @matched.match, diff --git a/lib/brakeman/checks/check_cross_site_scripting.rb b/lib/brakeman/checks/check_cross_site_scripting.rb index 679ed8381f32d5421814dc1b799b3fa9d8583a79..736e1024f3bc35ce7c22680eb0645f37e8032bac 100644 --- a/lib/brakeman/checks/check_cross_site_scripting.rb +++ b/lib/brakeman/checks/check_cross_site_scripting.rb @@ -104,16 +104,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck if input = has_immediate_user_input?(out) add_result exp - case input.type - when :params - message = "Unescaped parameter value" - when :cookies - message = "Unescaped cookie value" - when :request - message = "Unescaped request value" - else - message = "Unescaped user input value" - end + message = "Unescaped #{friendly_type_of input}" warn :template => @current_template, :warning_type => "Cross Site Scripting", @@ -194,15 +185,8 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck message = nil if @matched - case @matched.type - when :model - unless tracker.options[:ignore_model_output] - message = "Unescaped model attribute" - end - when :params - message = "Unescaped parameter value" - when :cookies - message = "Unescaped cookie value" + unless @matched.type and tracker.options[:ignore_model_output] + message = "Unescaped #{friendly_type_of @matched}" end if message and not duplicate? exp diff --git a/lib/brakeman/checks/check_file_access.rb b/lib/brakeman/checks/check_file_access.rb index 4d52a147ad965b3cc8982811422f0aba750c2a4e..f140b00b29e53eccb563944b0c8fb9be97647d19 100644 --- a/lib/brakeman/checks/check_file_access.rb +++ b/lib/brakeman/checks/check_file_access.rb @@ -48,20 +48,7 @@ class Brakeman::CheckFileAccess < Brakeman::BaseCheck end if match - case match.type - when :params - message = "Parameter" - when :cookies - message = "Cookie" - when :request - message = "Request" - when :model - message = "Model attribute" - else - message = "User input" - end - - message << " value used in file name" + message = "#{friendly_type_of(match).capitalize} used in file name" warn :result => result, :warning_type => "File Access", diff --git a/lib/brakeman/checks/check_link_to.rb b/lib/brakeman/checks/check_link_to.rb index a38d56292a0e349e98ba833f1456017c77557208..5faf75411bc2eb2c9895bbed0deae8a54918eb8c 100644 --- a/lib/brakeman/checks/check_link_to.rb +++ b/lib/brakeman/checks/check_link_to.rb @@ -68,14 +68,7 @@ class Brakeman::CheckLinkTo < Brakeman::CheckCrossSiteScripting input = has_immediate_user_input?(argument) return false unless input - case input.type - when :params - message = "Unescaped parameter value in link_to" - when :cookies - message = "Unescaped cookie value in link_to" - else - message = "Unescaped user input value in link_to" - end + message = "Unescaped #{friendly_type_of input} in link_to" warn_xss(result, message, input.match, CONFIDENCE[:high]) end @@ -96,15 +89,11 @@ class Brakeman::CheckLinkTo < Brakeman::CheckCrossSiteScripting # Check if we should warn about the matched result def check_matched(result, matched = nil) return false unless matched - message = nil + return false if matched.type == :model and not tracker.options[:ignore_model_output] - if matched.type == :model and not tracker.options[:ignore_model_output] - message = "Unescaped model attribute in link_to" - elsif matched.type == :params - message = "Unescaped parameter value in link_to" - end + message = "Unescaped #{friendly_type_of matched} in link_to" - message ? warn_xss(result, message, @matched.match, CONFIDENCE[:med]) : false + warn_xss(result, message, @matched.match, CONFIDENCE[:med]) end # Create a warn for this xss diff --git a/lib/brakeman/checks/check_link_to_href.rb b/lib/brakeman/checks/check_link_to_href.rb index 6b57a5d7eecaec6a1e7b361620669b8b57dce0ff..5e1e4828490839cbe7edb2abe7c81f6024251bfa 100644 --- a/lib/brakeman/checks/check_link_to_href.rb +++ b/lib/brakeman/checks/check_link_to_href.rb @@ -42,14 +42,7 @@ class Brakeman::CheckLinkToHref < Brakeman::CheckLinkTo if input = has_immediate_user_input?(url_arg) - case input.type - when :params - message = "Unsafe parameter value in link_to href" - when :cookies - message = "Unsafe cookie value in link_to href" - else - message = "Unsafe user input value in link_to href" - end + message = "Unsafe #{friendly_type_of input} in link_to href" unless duplicate? result add_result result diff --git a/lib/brakeman/checks/check_render.rb b/lib/brakeman/checks/check_render.rb index 1eb49d0623c25a6c301524971f289bcc4c040b0c..e6a9f91198b1386ecb6e59faaf7cc9c2d423c467 100644 --- a/lib/brakeman/checks/check_render.rb +++ b/lib/brakeman/checks/check_render.rb @@ -47,22 +47,9 @@ class Brakeman::CheckRender < Brakeman::BaseCheck return end - message = "Render path contains " - - case input.type - when :params - message << "parameter value" - when :cookies - message << "cookie value" - when :request - message << "request value" - when :model - #Skip models - return - else - message << "user input value" - end + return if input.type == :model #skip models + message = "Render path contains #{friendly_type_of input}" warn :result => result, :warning_type => "Dynamic Render Path", diff --git a/lib/brakeman/checks/check_symbol_dos.rb b/lib/brakeman/checks/check_symbol_dos.rb index ba46634bfe324322611c1273411cb3b67163bd6d..272d17aa1122bc0efc10f8b5833d95f60f912bc5 100644 --- a/lib/brakeman/checks/check_symbol_dos.rb +++ b/lib/brakeman/checks/check_symbol_dos.rb @@ -52,20 +52,7 @@ class Brakeman::CheckSymbolDoS < Brakeman::BaseCheck end if confidence - input_type = case input.type - when :params - "parameter value" - when :cookies - "cookies value" - when :request - "request value" - when :model - "model attribute" - else - "user input" - end - - message = "Symbol conversion from unsafe string (#{input_type})" + message = "Symbol conversion from unsafe string (#{friendly_type_of input})" warn :result => result, :warning_type => "Denial of Service", diff --git a/lib/brakeman/checks/check_unsafe_reflection.rb b/lib/brakeman/checks/check_unsafe_reflection.rb index a2fad5f8b7c86b44847d93ab66050d9959b2b244..bc86b27715a3142f8d0c937ff82d4971aadbed30 100644 --- a/lib/brakeman/checks/check_unsafe_reflection.rb +++ b/lib/brakeman/checks/check_unsafe_reflection.rb @@ -38,20 +38,7 @@ class Brakeman::CheckUnsafeReflection < Brakeman::BaseCheck end if confidence - input_type = case input.type - when :params - "parameter value" - when :cookies - "cookies value" - when :request - "request value" - when :model - "model attribute" - else - "user input" - end - - message = "Unsafe Reflection method #{method} called with #{input_type}" + message = "Unsafe Reflection method #{method} called with #{friendly_type_of input}" warn :result => result, :warning_type => "Remote Code Execution", diff --git a/lib/brakeman/checks/check_yaml_load.rb b/lib/brakeman/checks/check_yaml_load.rb index 97adc24e1d5d0f3860203f786f30eab3f8560e4b..1f6bc30e2fba135f9242f8cf2c70cb8c4ae72f77 100644 --- a/lib/brakeman/checks/check_yaml_load.rb +++ b/lib/brakeman/checks/check_yaml_load.rb @@ -28,20 +28,7 @@ class Brakeman::CheckYAMLLoad < Brakeman::BaseCheck end if confidence - input_type = case input.type - when :params - "parameter value" - when :cookies - "cookies value" - when :request - "request value" - when :model - "model attribute" - else - "user input" - end - - message = "YAML.#{method} called with #{input_type}" + message = "YAML.#{method} called with #{friendly_type_of input}" warn :result => result, :warning_type => "Remote Code Execution", diff --git a/test/tests/test_rails3.rb b/test/tests/test_rails3.rb index 6a7296169cdf275cc91012fd91c9dd3ccc712da0..f8b8aefdcec99f9b01064407a0e05ae02f63b51c 100644 --- a/test/tests/test_rails3.rb +++ b/test/tests/test_rails3.rb @@ -1036,7 +1036,7 @@ class Rails3Tests < Test::Unit::TestCase assert_warning :type => :warning, :warning_type => "Remote Code Execution", :line => 125, - :message => /^YAML\.load\ called\ with\ cookies\ value/, + :message => /^YAML\.load\ called\ with\ cookie\ value/, :confidence => 1, :file => /home_controller\.rb/ end @@ -1064,7 +1064,7 @@ class Rails3Tests < Test::Unit::TestCase assert_warning :type => :warning, :warning_type => "Remote Code Execution", :line => 131, - :message => /^YAML\.load_stream\ called\ with\ cookies\ val/, + :message => /^YAML\.load_stream\ called\ with\ cookie\ value/, :confidence => 0, :file => /home_controller\.rb/ end diff --git a/test/tests/test_rails31.rb b/test/tests/test_rails31.rb index f96636f49fee22195312dcf287529a13c2309a62..a886d224a0e0fb5d4473cea7f0e3d07a7d76de9b 100644 --- a/test/tests/test_rails31.rb +++ b/test/tests/test_rails31.rb @@ -718,7 +718,7 @@ class Rails31Tests < Test::Unit::TestCase assert_warning :type => :warning, :warning_type => "File Access", :line => 109, - :message => /^Model attribute\ value\ used\ in\ file\ name/, + :message => /^Model attribute\ used\ in\ file\ name/, :confidence => 1, :file => /users_controller\.rb/ end