diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index 4ffdd761f3fdeedd19972ea1246b98f8c3f3950a..d17e70501daf644cd7afc02d745520c65fbf9ce5 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -296,6 +296,9 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor when :if (sexp? exp[2] and has_immediate_user_input? exp[2]) or (sexp? exp[3] and has_immediate_user_input? exp[3]) + when :or + has_immediate_user_input? exp[1] or + has_immediate_user_input? exp[2] else false end diff --git a/test/apps/rails2/app/controllers/home_controller.rb b/test/apps/rails2/app/controllers/home_controller.rb index c374d84ea32f0072ca9097e72f5faaff19caa2ea..519826b740f6d02f25b86243df71934414694939 100644 --- a/test/apps/rails2/app/controllers/home_controller.rb +++ b/test/apps/rails2/app/controllers/home_controller.rb @@ -127,6 +127,18 @@ class HomeController < ApplicationController User.new(params[:still_bad]) end + def test_xss_with_or + @params_or_something = params[:x] || something + + if some_condition + @user_input = true + else + @user_input = params[:y] + end + + @more_user_input = x || params[:z] || z + end + private def filter_it diff --git a/test/apps/rails2/app/views/home/test_xss_with_or.html.erb b/test/apps/rails2/app/views/home/test_xss_with_or.html.erb new file mode 100644 index 0000000000000000000000000000000000000000..61d724f4328b92aff19d078865a8b96f3921d050 --- /dev/null +++ b/test/apps/rails2/app/views/home/test_xss_with_or.html.erb @@ -0,0 +1,7 @@ +<%= params[:x] || nil %> + +<%= @params_or_something %> + +<%= @user_input %> + +<%= @more_user_input %> diff --git a/test/tests/test_rails2.rb b/test/tests/test_rails2.rb index e6e0cf5966a02070a489666f447a3b768e6c9180..555bf54cf2bcccbbb0782cc60bd42c56c997e854 100644 --- a/test/tests/test_rails2.rb +++ b/test/tests/test_rails2.rb @@ -11,13 +11,13 @@ class Rails2Tests < Test::Unit::TestCase @expected ||= { :controller => 1, :model => 2, - :template => 27, + :template => 31, :warning => 29 } else @expected ||= { :controller => 1, :model => 2, - :template => 27, + :template => 31, :warning => 30 } end end @@ -593,6 +593,42 @@ class Rails2Tests < Test::Unit::TestCase :file => /home\/test_render_template\.html\.haml/ end + def test_xss_with_or_in_view + assert_warning :type => :template, + :warning_type => "Cross Site Scripting", + :line => 1, + :message => /^Unescaped\ parameter\ value/, + :confidence => 0, + :file => /test_xss_with_or\.html\.erb/ + end + + def test_xss_with_or_from_action + assert_warning :type => :template, + :warning_type => "Cross Site Scripting", + :line => 3, + :message => /^Unescaped\ parameter\ value/, + :confidence => 0, + :file => /test_xss_with_or\.html\.erb/ + end + + def test_xss_with_or_from_if_branches + assert_warning :type => :template, + :warning_type => "Cross Site Scripting", + :line => 5, + :message => /^Unescaped\ parameter\ value/, + :confidence => 0, + :file => /test_xss_with_or\.html\.erb/ + end + + def test_xss_with_nested_or + assert_warning :type => :template, + :warning_type => "Cross Site Scripting", + :line => 7, + :message => /^Unescaped\ parameter\ value/, + :confidence => 0, + :file => /test_xss_with_or\.html\.erb/ + end + def test_check_send assert_warning :type => :warning, :warning_type => "Dangerous Send",