Merge branch 'master' of github.com:presidentbeef/brakeman

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

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

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