Merge branch 'master' into better_file_name_handling_for_warnings

Conflicts:
	test/tests/test_rails3.rb

README.md

 ![Brakeman Logo](http://brakemanscanner.org/images/logo_medium.png)
 
-[![Travis CI Status](https://secure.travis-ci.org/presidentbeef/brakeman.png)](https://travis-ci.org/presidentbeef/brakeman) [![Code Climate](https://codeclimate.com/badge.png)](https://codeclimate.com/github/presidentbeef/brakeman)
+[![Travis CI
+Status](https://secure.travis-ci.org/presidentbeef/brakeman.png)](https://travis-ci.org/presidentbeef/brakeman)
+[![Code
+Climate](https://codeclimate.com/github/presidentbeef/brakeman.png)](https://codeclimate.com/github/presidentbeef/brakeman)
 
 # Brakeman
 
 Brakeman is a static analysis tool which checks Ruby on Rails applications for security vulnerabilities.
 
 It targets Rails versions 2.x and 3.x.
- 
+
 There is also a [plugin available](http://brakemanscanner.org/docs/jenkins/) for Jenkins/Hudson.
 
 For even more continuous testing, try the [Guard plugin](https://github.com/oreoshake/guard-brakeman).

lib/brakeman.rb

@@ -77,17 +77,10 @@ module Brakeman
     options
   end
 
-  DEPRECATED_CONFIG_FILES = [
-    File.expand_path("./config.yaml"),
-    File.expand_path("~/.brakeman/config.yaml"),
-    File.expand_path("/etc/brakeman/config.yaml"),
-    "#{File.expand_path(File.dirname(__FILE__))}/../lib/config.yaml"
-  ]
-
   CONFIG_FILES = [
     File.expand_path("./config/brakeman.yml"),
     File.expand_path("~/.brakeman/config.yml"),
-    File.expand_path("/etc/brakeman/config.yml"),
+    File.expand_path("/etc/brakeman/config.yml")
   ]
 
   #Load options from YAML file
@@ -103,12 +96,9 @@ module Brakeman
     end
   end
 
-  def self.config_file(custom_location=nil)
-    DEPRECATED_CONFIG_FILES.each do |f|
-      notify "#{f} is deprecated, please use one of #{CONFIG_FILES.join(", ")}" if File.file?(f)
-    end
-    supported_locations = [File.expand_path(custom_location || "")] + DEPRECATED_CONFIG_FILES + CONFIG_FILES
-    supported_locations.detect{|f| File.file?(f) }
+  def self.config_file custom_location = nil
+    supported_locations = [File.expand_path(custom_location || "")] + CONFIG_FILES
+    supported_locations.detect {|f| File.file?(f) }
   end
 
   #Default set of options

lib/brakeman/checks/base_check.rb

@@ -26,7 +26,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
     @active_record_models = nil
     @mass_assign_disabled = nil
     @has_user_input = nil
-    @safe_input_attributes = Set[:to_i, :to_f, :arel_table]
+    @safe_input_attributes = Set[:to_i, :to_f, :arel_table, :id]
   end
 
   #Add result to result list, which is used to check for duplicates

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -62,10 +62,17 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
     initializers = tracker.check_initializers :ActiveSupport, :escape_html_entities_in_json=
     initializers.each {|result| json_escape_on = true?(result.call.first_arg) }
 
+    if tracker.config[:rails][:active_support] and
+      true? tracker.config[:rails][:active_support][:escape_html_entities_in_json]
+
+        json_escape_on = true
+    end
+
     if !json_escape_on or version_between? "0.0.0", "2.0.99"
       @known_dangerous << :to_json
       Brakeman.debug("Automatic to_json escaping not enabled, consider to_json dangerous")
     else
+      @safe_input_attributes << :to_json
       Brakeman.debug("Automatic to_json escaping is enabled.")
     end
 

lib/brakeman/checks/check_model_attributes.rb

@@ -57,6 +57,7 @@ class Brakeman::CheckModelAttributes < Brakeman::BaseCheck
           warn :model => name,
             :file => model[:file],
             :warning_type => "Attribute Restriction",
+            :warning_code => :no_attr_accessible,
             :message => "Mass assignment is not restricted using attr_accessible",
             :confidence => CONFIDENCE[:high]
         elsif not tracker.options[:ignore_attr_protected]

lib/brakeman/processors/controller_processor.rb

@@ -24,6 +24,12 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
   def process_class exp
     name = class_name(exp.class_name)
 
+    if not name.to_s.match(/Controller$/)
+      #Skip classes that are not controllers, but treat as a module because
+      #a class that is not a controller might contain a controller
+      return process_module exp
+    end
+
     if @controller
       Brakeman.debug "[Notice] Skipping inner class: #{name}"
       return ignore

test/apps/rails2/app/views/home/_models.html.erb

+<%= model.id %>

test/apps/rails2/app/views/home/test_model.html.erb

@@ -8,4 +8,6 @@ Bad for 2.x: <%= link_to User.first.name, "some_url" %>
 
 It's just a model <%= link_to "Hipster ipsum", User.first %>
 
-It's just a couple of models <%= link_to "Hipster ipsum", [Account.first, User.last] %>
\ No newline at end of file
+It's just a couple of models <%= link_to "Hipster ipsum", [Account.first, User.last] %>
+
+<%= render :partial => 'models', :collection => all_the_things, :as => :model %>

test/apps/rails3.2/app/views/users/show.html.erb

@@ -18,3 +18,7 @@
 
 <%= link_to 'Edit', edit_user_path(@user) %> |
 <%= link_to 'Back', users_path %>
+
+<script>
+  var thing = <%= raw params.to_json %>;
+</script>

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

+class Whatever
+  module Wherever
+    class NestedController < ApplicationController
+      def so_nested
+        @bad_thing = params[:x]
+      end
+    end
+  end
+end

test/apps/rails3/app/views/whatever/wherever/nested/so_nested.html.erb

+<%= raw @bad_thing %>

test/tests/test_rails2.rb

@@ -293,6 +293,7 @@ class Rails2Tests < Test::Unit::TestCase
   def test_attribute_restriction
     assert_warning :type => :model,
       :warning_type => "Attribute Restriction",
+      :warning_code => Brakeman::WarningCodes::Codes[:no_attr_accessible],
       :message => /^Mass assignment is not restricted using /,
       :confidence => 0,
       :file => /account, user\.rb/
@@ -908,6 +909,16 @@ class Rails2Tests < Test::Unit::TestCase
       :file => /test_to_i\.html\.erb/
   end
 
+  def test_cross_site_scripting_unresolved_model_id
+    assert_no_warning :type => :template,
+      :warning_code => 2,
+      :warning_type => "Cross Site Scripting",
+      :line => 1,
+      :message => /^Unescaped\ model\ attribute/,
+      :confidence => 0,
+      :file => /_models\.html\.erb/
+  end
+
   def test_dangerous_send_try
     assert_warning :type => :warning,
       :warning_type => "Dangerous Send",
@@ -992,3 +1003,49 @@ class Rails2Tests < Test::Unit::TestCase
   end
 
 end
+
+Rails2WithOptions = BrakemanTester.run_scan "rails2", "Rails 2", :collapse_mass_assignment => false
+
+class Rails2WithOptionsTests < Test::Unit::TestCase
+  include BrakemanTester::FindWarning
+  include BrakemanTester::CheckExpected
+
+  def expected
+    if Brakeman::Scanner::RUBY_1_9
+      @expected ||= {
+        :controller => 1,
+        :model => 4,
+        :template => 43,
+        :warning => 45 }
+    else
+      @expected ||= {
+        :controller => 1,
+        :model => 4,
+        :template => 43,
+        :warning => 46 }
+    end
+  end
+
+  def report
+    Rails2WithOptions
+  end
+
+  def test_no_errors
+    assert_equal 0, report[:errors].length
+  end
+
+  def test_attribute_restriction
+    assert_warning :type => :model,
+      :warning_type => "Attribute Restriction",
+      :warning_code => Brakeman::WarningCodes::Codes[:no_attr_accessible],
+      :message => /^Mass assignment is not restricted using /,
+      :confidence => 0,
+      :file => /account\.rb/
+    assert_warning :type => :model,
+      :warning_type => "Attribute Restriction",
+      :warning_code => Brakeman::WarningCodes::Codes[:no_attr_accessible],
+      :message => /^Mass assignment is not restricted using /,
+      :confidence => 0,
+      :file => /user\.rb/
+  end
+end

test/tests/test_rails3.rb

@@ -14,7 +14,7 @@ class Rails3Tests < Test::Unit::TestCase
     @expected ||= {
       :controller => 1,
       :model => 8,
-      :template => 36,
+      :template => 37,
       :warning => 54
     }
 
@@ -834,6 +834,16 @@ class Rails3Tests < Test::Unit::TestCase
       :file => /test_params\.html\.erb/
   end
 
+  def test_cross_site_scripting_in_nested_controller
+    assert_warning :type => :template,
+      :warning_code => 2,
+      :warning_type => "Cross Site Scripting",
+      :line => 1,
+      :message => /^Unescaped\ parameter\ value/,
+      :confidence => 0,
+      :file => /so_nested\.html\.erb/
+  end
+
   def test_cross_site_scripting_select_tag_CVE_2012_3463
     assert_warning :type => :template,
       :warning_type => "Cross Site Scripting",

test/tests/test_rails32.rb

@@ -151,6 +151,15 @@ class Rails32Tests < Test::Unit::TestCase
       :file => /show\.html\.erb/
   end
 
+  def test_escaped_params_to_json
+    assert_no_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 21,
+      :message => /^Unescaped\ parameter\ value/,
+      :confidence => 0,
+      :file => /show\.html\.erb/
+  end
+
   def test_cross_site_scripting_in_slim_param
     assert_warning :type => :template,
       :warning_type => "Cross Site Scripting",