diff --git a/lib/brakeman/checks/check_redirect.rb b/lib/brakeman/checks/check_redirect.rb index 2619f8f3712d86881eb75731eba20840bfa8bfd4..9bceb6782774e768ae4e39be7242b43cfe0bf074 100644 --- a/lib/brakeman/checks/check_redirect.rb +++ b/lib/brakeman/checks/check_redirect.rb @@ -59,6 +59,10 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck args = call.args first_arg = call.first_arg + # if the first argument is an array, rails assumes you are building a + # polymorphic route, which will never jump off-host + return false if array? first_arg + if tracker.options[:ignore_redirect_to_model] and call? first_arg and (@model_find_calls.include? first_arg.method or first_arg.method.to_s.match(/^find_by_/)) and model_name? first_arg.target diff --git a/test/apps/rails2/app/controllers/home_controller.rb b/test/apps/rails2/app/controllers/home_controller.rb index 2b75358efe5c6d55861ca94ad5a3a8f14a24438b..8a4835e7a12bee6f3d66de132bbe8c7111eb82ea 100644 --- a/test/apps/rails2/app/controllers/home_controller.rb +++ b/test/apps/rails2/app/controllers/home_controller.rb @@ -160,4 +160,16 @@ class HomeController < ApplicationController def or_equals params[:still_bad] ||= {} end + + def test_safe_model_redirect + redirect_to User.find(1) + end + + def test_safe_mode_array_redirect + redirect_to [User.find(1), User.find(2)] + end + + def test_model_attributes_badness + redirect_to User.new.donkey + end end diff --git a/test/tests/test_rails2.rb b/test/tests/test_rails2.rb index 952b8500752e77e641065d795cbbd13b88bb62ac..86589fc90d05b1192346f268cee43be1a4c7b827 100644 --- a/test/tests/test_rails2.rb +++ b/test/tests/test_rails2.rb @@ -12,13 +12,13 @@ class Rails2Tests < Test::Unit::TestCase :controller => 1, :model => 2, :template => 41, - :warning => 31} + :warning => 32} else @expected ||= { :controller => 1, :model => 2, :template => 41, - :warning => 32 } + :warning => 33 } end end @@ -111,6 +111,13 @@ class Rails2Tests < Test::Unit::TestCase :message => /^Possible unprotected redirect/, :confidence => 0, :file => /home_controller\.rb/ + + assert_warning :type => :warning, + :warning_type => "Redirect", + :line => 173, + :message => /^Possible unprotected redirect/, + :confidence => 0, + :file => /home_controller\.rb/ end def test_dynamic_render_path