Merge branch 'master' into update_ruby_parser

Conflicts:
	lib/brakeman/checks/base_check.rb
	lib/brakeman/processors/model_processor.rb

CHANGES

+# 1.8.3
+
+ * Use `multi_json` gem for better harmony
+ * Performance improvement for call indexing
+ * Fix issue with processing HAML files
+ * Handle pre-release versions when processing `Gemfile.lock`
+ * Only check first argument of `redirect_to`
+ * Fix false positives from `Model.arel_table` accesses
+ * Fix false positives on redirects to models decorated with Draper gem
+ * Fix false positive on redirect to model association
+ * Fix false positive on `YAML.load`
+ * Fix false positive XSS on any `to_i` output
+ * Fix error on Rails 2 name routes with no args
+ * Fix error in rescan of mixins with symbols in method name
+ * Do not rescan non-Ruby files in config/
+
 # 1.8.2
 
  * Fixed rescanning problems caused by 1.8.0 changes

bin/brakeman

@@ -54,7 +54,8 @@ end
 
 if options[:previous_results_json]
   vulns = Brakeman.compare options.merge(:quiet => options[:quiet])
-  puts JSON.pretty_generate(vulns)
+  puts MultiJson.dump(vulns, :pretty => true)
+
   if options[:exit_on_warn] and (vulns[:new].count + vulns[:fixed].count > 0)
     exit Brakeman::Warnings_Found_Exit_Code
   end

brakeman.gemspec

@@ -19,5 +19,5 @@ Gem::Specification.new do |s|
   s.add_dependency "erubis", "~>2.6"
   s.add_dependency "haml", "~>3.0"
   s.add_dependency "sass", "~>3.0"
-  s.add_dependency "json_pure"
+  s.add_dependency "multi_json", "~>1.3"
 end

lib/brakeman.rb

@@ -314,19 +314,20 @@ module Brakeman
 
   # Compare JSON ouptut from a previous scan and return the diff of the two scans
   def self.compare options
-    require 'json'
+    require 'multi_json'
     require 'brakeman/differ'
     raise ArgumentError.new("Comparison file doesn't exist") unless File.exists? options[:previous_results_json]
 
     begin
-      previous_results = JSON.parse(File.read(options[:previous_results_json]), :symbolize_names =>true)[:warnings]
-    rescue JSON::ParserError
+      previous_results = MultiJson.load(File.read(options[:previous_results_json]), :symbolize_keys => true)[:warnings]
+    rescue MultiJson::DecodeError
       self.notify "Error parsing comparison file: #{options[:previous_results_json]}"
       exit!
     end
 
     tracker = run(options)
-    new_results = JSON.parse(tracker.report.to_json, :symbolize_names =>true)[:warnings]
+
+    new_results = MultiJson.load(tracker.report.to_json, :symbolize_keys => true)[:warnings]
 
     Brakeman::Differ.new(new_results, previous_results).diff
   end

lib/brakeman/call_index.rb

@@ -106,7 +106,7 @@ class Brakeman::CallIndex
   def index_calls calls
     calls.each do |call|
       @methods << call[:method].to_s
-      @targets << call[:target].to_s
+      @targets << call[:target].to_s if call[:target].is_a? Symbol
       @calls_by_method[call[:method]] << call
       @calls_by_target[call[:target]] << call
     end
@@ -138,7 +138,7 @@ class Brakeman::CallIndex
       elsif targets.length > 1
         calls_by_targets targets
       else
-        calls_by_target[targets.first]
+        @calls_by_target[targets.first]
       end
     else
       @calls_by_target[target]

lib/brakeman/checks/base_check.rb

@@ -23,6 +23,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
     @current_set = nil
     @current_template = @current_module = @current_class = @current_method = nil
     @mass_assign_disabled = nil
+    @safe_input_attributes = Set[:to_i, :to_f, :arel_table]
   end
 
   #Add result to result list, which is used to check for duplicates
@@ -63,14 +64,16 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
 
     target = exp.target
 
-    if params? target
-      @has_user_input = Match.new(:params, exp)
-    elsif cookies? target
-      @has_user_input = Match.new(:cookies, exp)
-    elsif request_env? target
-      @has_user_input = Match.new(:request, exp)
-    elsif sexp? target and model_name? target[1] #TODO: Can this be target.target?
-      @has_user_input = Match.new(:model, exp)
+    unless @safe_input_attributes.include? exp.method
+      if params? target
+        @has_user_input = Match.new(:params, exp)
+      elsif cookies? target
+        @has_user_input = Match.new(:cookies, exp)
+      elsif request_env? target
+        @has_user_input = Match.new(:request, exp)
+      elsif sexp? target and model_name? target[1] #TODO: Can this be target.target?
+        @has_user_input = Match.new(:model, exp)
+      end
     end
 
     exp
@@ -255,19 +258,15 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
   def has_immediate_user_input? exp
     if exp.nil?
       false
-    elsif params? exp
-      return Match.new(:params, exp)
-    elsif cookies? exp
-      return Match.new(:cookies, exp)
-    elsif call? exp
-      if params? exp.target
+    elsif call? exp and not @safe_input_attributes.include? exp.method
+      if params? exp
         return Match.new(:params, exp)
-      elsif cookies? exp.target
+      elsif cookies? exp
         return Match.new(:cookies, exp)
-      elsif request_env? exp.target
+      elsif request_env? exp
         return Match.new(:request, exp)
       else
-        false
+        has_immediate_user_input? exp.target
       end
     elsif sexp? exp
       case exp.node_type
@@ -318,9 +317,11 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
       target = exp.target
       method = exp.method
 
-      if call? target and not method.to_s[-1,1] == "?"
+      if @safe_input_attributes.include? method
+        false
+      elsif call? target and not method.to_s[-1,1] == "?"
         has_immediate_model? target, out
-      elsif model_name? target and method != :arel_table
+      elsif model_name? target
         exp
       else
         false

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -236,6 +236,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
       ((target == URI or target == CGI) and method == :escape) or
       (target == XML_HELPER and method == :escape_xml) or
       (target == FORM_BUILDER and @ignore_methods.include? method) or
+      (target and @safe_input_attributes.include? method) or
       (method.to_s[-1,1] == "?")
 
       #exp[0] = :ignore #should not be necessary

lib/brakeman/checks/check_file_access.rb

@@ -9,7 +9,9 @@ class Brakeman::CheckFileAccess < Brakeman::BaseCheck
 
   def run_check
     Brakeman.debug "Finding possible file access"
-    methods = tracker.find_call :targets => [:Dir, :File, :IO, :Kernel, :"Net::FTP", :"Net::HTTP", :PStore, :Pathname, :Shell, :YAML], :methods => [:[], :chdir, :chroot, :delete, :entries, :foreach, :glob, :install, :lchmod, :lchown, :link, :load, :load_file, :makedirs, :move, :new, :open, :read, :readlines, :rename, :rmdir, :safe_unlink, :symlink, :syscopy, :sysopen, :truncate, :unlink]
+    methods = tracker.find_call :targets => [:Dir, :File, :IO, :Kernel, :"Net::FTP", :"Net::HTTP", :PStore, :Pathname, :Shell], :methods => [:[], :chdir, :chroot, :delete, :entries, :foreach, :glob, :install, :lchmod, :lchown, :link, :load, :load_file, :makedirs, :move, :new, :open, :read, :readlines, :rename, :rmdir, :safe_unlink, :symlink, :syscopy, :sysopen, :truncate, :unlink]
+
+    methods.concat tracker.find_call :target => :YAML, :methods => [:load_file, :parse_file]
 
     Brakeman.debug "Finding calls to load()"
     methods.concat tracker.find_call :target => false, :method => :load

lib/brakeman/checks/check_redirect.rb

@@ -53,44 +53,40 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck
   #is being output directly. This is necessary because of tracker.options[:check_arguments]
   #which can be used to enable/disable reporting output of method calls which use
   #user input as arguments.
-  def include_user_input? call
+  def include_user_input? call, immediate = :immediate
     Brakeman.debug "Checking if call includes user input"
 
-    args = call.args
-    first_arg = call.first_arg
+    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
+    return false if array? arg
 
-      return false
+    if tracker.options[:ignore_redirect_to_model]
+      if model_instance?(arg) or decorated_model?(arg)
+        return false
+      end
     end
 
-    args.each do |arg|
-      if res = has_immediate_model?(arg)
-        return Match.new(:immediate, res)
-      elsif call? arg
-        if request_value? arg
-          return Match.new(:immediate, arg)
-        elsif request_value? arg[1]
-          return Match.new(:immediate, arg[1])
-        elsif arg[2] == :url_for and include_user_input? arg
-          return Match.new(:immediate, arg)
-          #Ignore helpers like some_model_url?
-        elsif arg[2].to_s =~ /_(url|path)$/
-          return false
-        end
-      elsif request_value? arg
-        return Match.new(:immediate, arg)
+    if res = has_immediate_model?(arg)
+      return Match.new(immediate, res)
+    elsif call? arg
+      if request_value? arg
+        return Match.new(immediate, arg)
+      elsif request_value? arg[1]
+        return Match.new(immediate, arg[1])
+      elsif arg[2] == :url_for and include_user_input? arg
+        return Match.new(immediate, arg)
+        #Ignore helpers like some_model_url?
+      elsif arg[2].to_s =~ /_(url|path)\z/
+        return false
       end
+    elsif request_value? arg
+      return Match.new(immediate, arg)
     end
 
-    if tracker.options[:check_arguments]
-      super
+    if tracker.options[:check_arguments] and call? arg
+      include_user_input? arg, false  #I'm doubting if this is really necessary...
     else
       false
     end
@@ -125,4 +121,56 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck
 
     true
   end
+
+  #Returns true if exp is (probably) a model instance
+  def model_instance? exp
+    if node_type? exp, :or
+      model_instance? exp.lhs or model_instance? exp.rhs
+    elsif call? exp
+      if model_name? exp.target and
+        (@model_find_calls.include? exp.method or exp.method.to_s.match(/^find_by_/))
+        true
+      else
+        association?(exp.target, exp.method)
+      end
+    end
+  end
+
+  #Returns true if exp is (probably) a decorated model instance
+  #using the Draper gem
+  def decorated_model? exp
+    if node_type? exp, :or
+      decorated_model? exp.lhs or decorated_model? exp.rhs
+    else
+      tracker.config[:gems] and
+      tracker.config[:gems][:draper] and
+      call? exp and
+      node_type?(exp.target, :const) and
+      exp.target.value.to_s.match(/Decorator$/) and
+      exp.method == :decorate
+    end
+  end
+
+  #Check if method is actually an association in a Model
+  def association? model_name, meth
+    if call? model_name
+      return association? model_name.target, meth
+    elsif model_name? model_name
+      model = tracker.models[class_name(model_name)]
+    else
+      return false
+    end
+
+    return false unless model
+
+    model[:associations].each do |name, args|
+      args.each do |arg|
+        if symbol? arg and arg.value == meth
+          return true
+        end
+      end
+    end
+
+    false
+  end
 end

lib/brakeman/processors/haml_template_processor.rb

@@ -74,7 +74,8 @@ class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor
       res
 
       #_hamlout.buffer <<
-      #This seems to be used rarely, but directly appends args to output buffer
+      #This seems to be used rarely, but directly appends args to output buffer.
+      #Has something to do with values of blocks?
     elsif sexp? target and method == :<< and is_buffer_target? target
       @inside_concat = true
       out = exp.arglist[1] = process(exp.arglist[1])

lib/brakeman/processors/lib/rails2_route_processor.rb

@@ -112,7 +112,8 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
     hash_iterate(exp) do |option, value|
       case option[1]
       when :controller, :requirements, :singular, :path_prefix, :as,
-        :path_names, :shallow, :name_prefix, :member_path, :nested_member_path
+        :path_names, :shallow, :name_prefix, :member_path, :nested_member_path,
+        :belongs_to, :conditions, :active_scaffold
         #should be able to skip
       when :collection, :member, :new
         process_collection value
@@ -129,7 +130,7 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
       when :except
         process_option_except value
       else
-        Brakeman.notify "[Notice] Unhandled resource option: #{option}"
+        Brakeman.notify "[Notice] Unhandled resource option, please report: #{option}"
       end
     end
   end
@@ -178,6 +179,8 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
   #Process
   # map.connect '/something', :controller => 'blah', :action => 'whatever'
   def process_connect exp
+    return if exp.empty?
+
     controller = check_for_controller_name exp
     self.current_controller = controller if controller
     

lib/brakeman/processors/model_processor.rb

@@ -2,6 +2,9 @@ require 'brakeman/processors/base_processor'
 
 #Processes models. Puts results in tracker.models
 class Brakeman::ModelProcessor < Brakeman::BaseProcessor
+
+  ASSOCIATIONS = Set[:belongs_to, :has_one, :has_many, :has_and_belongs_to_many]
+
   def initialize tracker
     super 
     @model = nil
@@ -38,6 +41,7 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
         :private => {},
         :protected => {},
         :options => {},
+        :associations => {},
         :file => @file_name }
       @tracker.models[@model[:name]] = @model
       exp.body = process_all! exp.body
@@ -83,8 +87,13 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
           @model[:attr_accessible].concat args
         else
           if @model
-            @model[:options][method] ||= []
-            @model[:options][method] << exp.arglist.line(exp.line)
+            if ASSOCIATIONS.include? method
+              @model[:associations][method] ||= []
+              @model[:associations][method].concat exp.args
+            else
+              @model[:options][method] ||= []
+              @model[:options][method] << exp.arglist.line(exp.line)
+            end
           end
         end
       end

lib/brakeman/processors/template_alias_processor.rb

@@ -96,11 +96,17 @@ class Brakeman::TemplateAliasProcessor < Brakeman::AliasProcessor
     false
   end
 
+  #Ignore `<<` calls on template variables which are used by the templating
+  #library (HAML, ERB, etc.)
   def find_push_target exp
     if sexp? exp
       if exp.node_type == :lvar and (exp.value == :_buf or exp.value == :_erbout)
         return nil
       elsif exp.node_type == :ivar and exp.value == :@output_buffer
+        return nil
+      elsif exp.node_type == :call and call? exp.target and
+        exp.target.method == :_hamlout and exp.method == :buffer
+
         return nil
       end
     end

lib/brakeman/report.rb

@@ -6,6 +6,7 @@ require 'brakeman/util'
 require 'terminal-table'
 require 'highline/system_extensions'
 require "csv"
+require 'multi_json'
 require 'brakeman/version'
 
 if CSV.const_defined? :Reader
@@ -17,6 +18,15 @@ else
   # CSV is now FasterCSV in ruby 1.9
 end
 
+#This is so OkJson will work with symbol values
+if MultiJson.default_adapter == :ok_json
+  class Symbol
+    def to_json
+      self.to_s.inspect
+    end
+  end
+end
+
 #Generates a report based on the Tracker and the results of
 #Tracker#run_checks. Be sure to +run_checks+ before generating
 #a report.
@@ -647,8 +657,6 @@ class Brakeman::Report
   end
 
   def to_json
-    require 'json'
-
     errors = tracker.errors.map{|e| { :error => e[:error], :location => e[:backtrace][0] }}
     app_path = tracker.options[:app_path]
 
@@ -662,7 +670,7 @@ class Brakeman::Report
       :app_path => File.expand_path(tracker.options[:app_path]),
       :rails_version => rails_version,
       :security_warnings => all_warnings.length,
-      :timestamp => Time.now,
+      :timestamp => Time.now.to_s,
       :checks_performed => checks.checks_run.sort,
       :number_of_controllers =>tracker.controllers.length,
       # ignore the "fake" model
@@ -672,11 +680,11 @@ class Brakeman::Report
       :brakeman_version => Brakeman::Version
     }
 
-    JSON.pretty_generate({
+    MultiJson.dump({
       :scan_info => scan_info,
       :warnings => warnings,
       :errors => errors
-    })
+    }, :pretty => true)
   end
 
   def all_warnings

lib/brakeman/rescanner.rb

@@ -304,7 +304,7 @@ class Brakeman::Rescanner < Brakeman::Scanner
       :initializer
     when /config\/routes\.rb/
       :routes
-    when /\/config/
+    when /\/config\/.+\.rb/
       :config
     when /Gemfile/
       :gemfile
@@ -322,7 +322,7 @@ class Brakeman::Rescanner < Brakeman::Scanner
       end
     end
 
-    method_matcher = /##{method_names.join('|')}$/
+    method_matcher = /##{method_names.map {|n| Regexp.escape(n.to_s)}.join('|')}$/
 
     #Rescan controllers that mixed in library
     tracker.controllers.each do |name, controller|

lib/brakeman/version.rb

 module Brakeman
-  Version = "1.8.2"
+  Version = "1.8.3"
 end

lib/brakeman/warning.rb

+require 'multi_json'
+
 #The Warning class stores information about warnings
 class Brakeman::Warning
   attr_reader :called_from, :check, :class, :confidence, :controller,
@@ -177,8 +179,6 @@ class Brakeman::Warning
   end
 
   def to_json
-    require 'json'
-
-    JSON.dump self.to_hash
+    MultiJson.dump self.to_hash
   end
 end

test/apps/rails2/app/controllers/other_controller.rb

@@ -37,4 +37,9 @@ class OtherController < ApplicationController
       do_something
     end
   end
+
+  def test_to_i
+    @x = params[:x].to_i
+    @id = cookies[:id].to_i
+  end
 end

test/apps/rails2/app/views/other/test_to_i.html.erb

+<%= @x %>
+
+<%= request.env[:QUERY_STRING].to_i %>
+
+<%= out @id %>
+
+<%= User.current.age.to_i %>
+
+<%= out Account.current.number.to_i %>

test/apps/rails2/config/routes.rb

@@ -53,4 +53,5 @@ ActionController::Routing::Routes.draw do |map|
   # consider removing or commenting them out if you're using named routes and resources.
   map.connect ':controller/:action/:id'
   map.connect ':controller/:action/:id.:format'
+  map.some_routes
 end

test/apps/rails3.1/Gemfile

@@ -11,6 +11,8 @@ gem 'json'
 
 gem 'therubyracer'
 
+gem 'draper'
+
 # Gems used only for assets and not required
 # in production environments by default.
 group :assets do

test/apps/rails3.1/app/controllers/other_controller.rb

@@ -45,4 +45,18 @@ class OtherController < ApplicationController
   def test_arel_table_access
     User.where(User.arel_table[:id].eq(params[:some_id]))
   end
+
+  def test_draper_redirect
+    redirect_to RecordDecorator.decorate(Record.where(:something => params[:access_key]).find(params[:id]))
+  end
+
+  def test_model_redirect_in_or
+    if something
+      user = User.find(params[:something])
+    else
+      user = User.find(params[:else])
+    end
+
+    redirect_to user
+  end
 end

test/apps/rails3.1/app/controllers/users_controller.rb

@@ -109,5 +109,17 @@ class UsersController < ApplicationController
     IO.read(User.find_by_name('bob').file_path)
   end
 
+  def redirect_to_user_as_param
+    redirect_to blah(User.find(1)) #Don't warn
+  end
+
+  def redirect_to_association
+    redirect_to User.first.account #Don't warn
+  end
+
+  def redirect_to_safe_second_param
+    redirect_to :back, :notice => "Go back, #{params[:user]}!" #Don't warn
+  end
+
   include UserMixin
 end

test/apps/rails3.1/app/models/user.rb

@@ -23,4 +23,6 @@ class User < ActiveRecord::Base
   }
 
   scope :all
+
+  belongs_to :account
 end

test/apps/rails3.2/lib/user_controller_mixin.rb

@@ -2,4 +2,7 @@ module UserControllerMixin
   def mixed_in
     @user = User.find(params[:id])
   end
+
+  def [] index
+  end
 end

test/apps/rails3/app/controllers/home_controller.rb

@@ -101,6 +101,14 @@ class HomeController < ApplicationController
     @user = User.find(current_user)
   end
 
+  def test_yaml_file_access
+    #Should not warn
+    YAML.load "some/path/#{params[:user][:file]}"
+
+    #Should warn
+    YAML.parse_file("whatever/" + params[:file_name])
+  end
+
   private
 
   def filter_it

test/tests/test_json_compare.rb

@@ -6,7 +6,7 @@ class JSONCompareTests < Test::Unit::TestCase
     @json_path = File.join @path, "report.json"
     File.delete @json_path if File.exist? @json_path
     Brakeman.run :app_path => @path, :output_files => [@json_path]
-    @report = JSON.parse File.read(@json_path)
+    @report = MultiJson.load File.read(@json_path)
   end
 
   def update_json

test/tests/test_rails2.rb

@@ -788,5 +788,40 @@ class Rails2Tests < Test::Unit::TestCase
       :confidence => 0,
       :file => /test_to_json\.html\.erb/
   end
-end
 
+  def test_xss_with_params_to_i
+    assert_no_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 1,
+      :message => /^Unescaped\ parameter\ value/,
+      :confidence => 0,
+      :file => /test_to_i\.html\.erb/
+  end
+
+  def test_xss_with_request_env_to_i
+    assert_no_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 5,
+      :message => /^Unescaped\ cookie\ value/,
+      :confidence => 2,
+      :file => /test_to_i\.html\.erb/
+  end
+
+  def test_xss_with_cookie_to_i
+    assert_no_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 3,
+      :message => /^Unescaped\ request\ value/,
+      :confidence => 0,
+      :file => /test_to_i\.html\.erb/
+  end
+
+  def test_xss_with_model_attribute_to_i
+    assert_no_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 7,
+      :message => /^Unescaped\ model\ attribute/,
+      :confidence => 1,
+      :file => /test_to_i\.html\.erb/
+  end
+end

test/tests/test_rails3.rb

@@ -15,7 +15,7 @@ class Rails3Tests < Test::Unit::TestCase
       :controller => 1,
       :model => 5,
       :template => 30,
-      :warning => 30
+      :warning => 31
     }
   end
 
@@ -81,6 +81,24 @@ class Rails3Tests < Test::Unit::TestCase
       :file => /home_controller\.rb/
   end
 
+  def test_file_access_yaml_load
+    assert_no_warning :type => :warning,
+      :warning_type => "File Access",
+      :line => 106,
+      :message => /^Parameter\ value\ used\ in\ file\ name/,
+      :confidence => 0,
+      :file => /home_controller\.rb/
+  end
+
+  def test_file_access_yaml_parse_file
+    assert_warning :type => :warning,
+      :warning_type => "File Access",
+      :line => 109,
+      :message => /^Parameter\ value\ used\ in\ file\ name/,
+      :confidence => 0,
+      :file => /home_controller\.rb/
+  end
+
   def test_mass_assignment
     assert_warning :type => :warning,
       :warning_type => "Mass Assignment",

test/tests/test_rails31.rb

@@ -54,6 +54,51 @@ class Rails31Tests < Test::Unit::TestCase
       :file => /users_controller\.rb/
   end
 
+  def test_redirect_to_decorated_model
+    assert_no_warning :type => :warning,
+      :warning_type => "Redirect",
+      :line => 50,
+      :message => /^Possible\ unprotected\ redirect/,
+      :confidence => 2,
+      :file => /other_controller\.rb/
+  end
+
+  def test_redirect_multiple_values
+    assert_no_warning :type => :warning,
+      :warning_type => "Redirect",
+      :line => 61,
+      :message => /^Possible\ unprotected\ redirect/,
+      :confidence => 0,
+      :file => /other_controller\.rb/
+  end
+
+  def test_redirect_to_model_as_arg
+    assert_no_warning :type => :warning,
+      :warning_type => "Redirect",
+      :line => 113,
+      :message => /^Possible\ unprotected\ redirect/,
+      :confidence => 2,
+      :file => /users_controller\.rb/
+  end
+
+  def test_redirect_to_model_association
+    assert_no_warning :type => :warning,
+      :warning_type => "Redirect",
+      :line => 117,
+      :message => /^Possible\ unprotected\ redirect/,
+      :confidence => 0,
+      :file => /users_controller\.rb/
+  end
+
+  def test_redirect_to_secong_arg
+    assert_no_warning :type => :warning,
+      :warning_type => "Redirect",
+      :line => 121,
+      :message => /^Possible\ unprotected\ redirect/,
+      :confidence => 2,
+      :file => /users_controller\.rb/
+  end
+
   def test_whitelist_attributes
     assert_no_warning :type => :model,
       :warning_type => "Attribute Restriction",