Merge branch 'master' into update_ruby_parser

.gitignore

+Gemfile.lock

Gemfile

 source "http://rubygems.org"
 
 gemspec
+
+gem "rake"

README.md

@@ -145,32 +145,8 @@ Brakeman options can stored and read from YAML files. To simplify the process of
 
 Options passed in on the commandline have priority over configuration files.
 
-The default config locations are `./config.yaml`, `~/.brakeman/`, and `/etc/brakeman/config.yaml`
+The default config locations are `./config/brakeman.yml`, `~/.brakeman/config.yml`, and `/etc/brakeman/config.yml`
 
 The `-c` option can be used to specify a configuration file to use.
 
-# License
-
-The MIT License
-
-Copyright (c) 2012, Twitter, Inc.
-
-Copyright (c) 2010-2012, YELLOWPAGES.COM, LLC
-
-Permission is hereby granted, free of charge, to any person obtaining a copy
-of this software and associated documentation files (the "Software"), to deal
-in the Software without restriction, including without limitation the rights
-to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the Software is
-furnished to do so, subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in
-all copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
-THE SOFTWARE.
+# License see MIT-LICENSE

Rakefile

+task :default do
+  sh "cd test && ruby test.rb"
+end

brakeman.gemspec

@@ -9,6 +9,7 @@ Gem::Specification.new do |s|
   s.homepage = "http://brakemanscanner.org"
   s.files = ["bin/brakeman", "CHANGES", "WARNING_TYPES", "FEATURES", "README.md"] + Dir["lib/**/*"]
   s.executables = ["brakeman"]
+  s.license = "MIT"
   s.add_dependency "activesupport"
   s.add_dependency "i18n"
   s.add_dependency "ruby_parser", "~>3.0.4"

lib/brakeman.rb

@@ -80,31 +80,38 @@ module Brakeman
     options
   end
 
-  #Load options from YAML file
-  def self.load_options config_file
-    config_file ||= ""
+  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"),
+  ]
 
+  #Load options from YAML file
+  def self.load_options custom_location
     #Load configuration file
-    [File.expand_path(config_file),
-      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"].each do |f|
-
-      if File.exist? f and not File.directory? f
-        notify "[Notice] Using configuration in #{f}"
-        options = YAML.load_file f
-        options.each do |k,v|
-          if v.is_a? Array
-            options[k] = Set.new v
-          end
-        end
-
-        return options
-      end
-      end
+    if config = config_file(custom_location)
+      notify "[Notice] Using configuration in #{config}"
+      options = YAML.load_file config
+      options.each { |k, v| options[k] = Set.new v if v.is_a? Array }
+      options
+    else
+      {}
+    end
+  end
 
-    return {}
+  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) }
   end
 
   #Default set of options

lib/brakeman/call_index.rb

@@ -7,8 +7,6 @@ class Brakeman::CallIndex
   def initialize calls
     @calls_by_method = Hash.new { |h,k| h[k] = [] }
     @calls_by_target = Hash.new { |h,k| h[k] = [] }
-    @methods = Set.new
-    @targets = Set.new
 
     index_calls calls
   end
@@ -25,7 +23,7 @@ class Brakeman::CallIndex
     target = options[:target] || options[:targets]
     method = options[:method] || options[:methods]
     nested = options[:nested]
-    
+
     if options[:chained]
       return find_chain options
     #Find by narrowest category
@@ -72,16 +70,12 @@ class Brakeman::CallIndex
       calls.delete_if do |call|
         from_template call, template_name
       end
-
-      @methods.delete name.to_s if calls.empty?
     end
 
     @calls_by_target.each do |name, calls|
       calls.delete_if do |call|
         from_template call, template_name
       end
-
-      @targets.delete name.to_s if calls.empty?
     end
   end
 
@@ -90,25 +84,22 @@ class Brakeman::CallIndex
       calls.delete_if do |call|
         call[:location][0] == :class and classes.include? call[:location][1]
       end
-
-      @methods.delete name.to_s if calls.empty?
     end
 
     @calls_by_target.each do |name, calls|
       calls.delete_if do |call|
         call[:location][0] == :class and classes.include? call[:location][1]
       end
-
-      @targets.delete name.to_s if calls.empty?
     end
   end
 
   def index_calls calls
     calls.each do |call|
-      @methods << call[:method].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
+
+      unless call[:target].is_a? Sexp
+        @calls_by_target[call[:target]] << call
+      end
     end
   end
 
@@ -128,18 +119,6 @@ class Brakeman::CallIndex
   def calls_by_target target
     if target.is_a? Array
       calls_by_targets target
-    elsif target.is_a? Regexp
-      targets = @targets.select do |t|
-        t.match target
-      end
-
-      if targets.empty?
-        []
-      elsif targets.length > 1
-        calls_by_targets targets
-      else
-        @calls_by_target[targets.first]
-      end
     else
       @calls_by_target[target]
     end
@@ -158,18 +137,6 @@ class Brakeman::CallIndex
   def calls_by_method method
     if method.is_a? Array
       calls_by_methods method
-    elsif method.is_a? Regexp
-      methods = @methods.select do |m|
-        m.match method
-      end
-
-      if methods.empty?
-        []
-      elsif methods.length > 1
-        calls_by_methods methods
-      else
-        @calls_by_method[methods.first.to_sym]
-      end
     else
       @calls_by_method[method.to_sym]
     end

lib/brakeman/checks/check_translate_bug.rb

 require 'brakeman/checks/base_check'
 
 #Check for vulnerability in translate() helper that allows cross-site scripting
-#http://groups.google.com/group/rubyonrails-security/browse_thread/thread/2b61d70fb73c7cc5
 class Brakeman::CheckTranslateBug < Brakeman::BaseCheck
   Brakeman::Checks.add self
 
@@ -12,32 +11,34 @@ class Brakeman::CheckTranslateBug < Brakeman::BaseCheck
         version_between?('3.0.0', '3.0.10') or
         version_between?('3.1.0', '3.1.1')
 
-      if uses_translate?
-        confidence = CONFIDENCE[:high]
+      confidence = if uses_translate?
+        CONFIDENCE[:high]
       else
-        confidence = CONFIDENCE[:med]
+        CONFIDENCE[:med]
       end
 
       version = tracker.config[:rails_version]
+      description = "have a vulnerability in the translate helper with keys ending in _html"
 
-      if version =~ /^3\.1/
-        message = "Versions before 3.1.2 have a vulnerability in the translate helper."
+      message = if version =~ /^3\.1/
+        "Versions before 3.1.2 #{description}."
       elsif version =~ /^3\.0/
-        message = "Versions before 3.0.11 have a vulnerability in translate helper."
+        "Versions before 3.0.11 #{description}."
       else
-        message = "Rails 2.3.x using the rails_xss plugin have a vulnerability in translate helper."
+        "Rails 2.3.x using the rails_xss plugin #{description}}."
       end
 
       warn :warning_type => "Cross Site Scripting",
         :message => message,
         :confidence => confidence,
-        :file => gemfile_or_environment
+        :file => gemfile_or_environment,
+        :link_path => "http://groups.google.com/group/rubyonrails-security/browse_thread/thread/2b61d70fb73c7cc5"
     end
   end
 
   def uses_translate?
     Brakeman.debug "Finding calls to translate() or t()"
 
-    not tracker.find_call(:target => nil, :methods => [:t, :translate]).empty?
+    tracker.find_call(:target => nil, :methods => [:t, :translate]).any?
   end
 end

lib/brakeman/report.rb

@@ -475,7 +475,17 @@ class Brakeman::Report
 
   #Generate header for text output
   def text_header
-    "\n+BRAKEMAN REPORT+\n\nApplication path: #{File.expand_path tracker.options[:app_path]}\nRails version: #{rails_version}\nGenerated at #{Time.now}\nChecks run: #{checks.checks_run.sort.join(", ")}\n"
+    <<-HEADER
+
++BRAKEMAN REPORT+
+
+Application path: #{File.expand_path tracker.options[:app_path]}
+Rails version: #{rails_version}
+Brakeman version: #{Brakeman::Version}
+Started at #{tracker.start_time}
+Duration: #{tracker.duration} seconds
+Checks run: #{checks.checks_run.sort.join(", ")}
+HEADER
   end
 
   #Generate header for CSV output
@@ -670,7 +680,10 @@ class Brakeman::Report
       :app_path => File.expand_path(tracker.options[:app_path]),
       :rails_version => rails_version,
       :security_warnings => all_warnings.length,
-      :timestamp => Time.now.to_s,
+      :start_time => tracker.start_time.to_s,
+      :end_time => tracker.end_time.to_s,
+      :timestamp => tracker.end_time.to_s,
+      :duration => tracker.duration,
       :checks_performed => checks.checks_run.sort,
       :number_of_controllers =>tracker.controllers.length,
       # ignore the "fake" model

lib/brakeman/templates/header.html.erb

@@ -26,13 +26,18 @@
   <tr>
     <th>Application Path</th>
     <th>Rails Version</th>
-    <th>Report Generation Time</th>
+    <th>Brakeman Version</th>
+    <th>Report Time</th>
     <th>Checks Performed</th>
   </tr>
   <tr>
     <td><%= File.expand_path tracker.options[:app_path] %></td>
     <td><%= rails_version %></td>
-    <td><%= Time.now %></td>
+    <td><%= Brakeman::Version %>
+    <td>
+      <%= tracker.start_time %><br><br>
+      <%= tracker.duration %> seconds
+    </td>
     <td><%= checks.checks_run.sort.join(", ") %></td>
   </tr>
 </table>

lib/brakeman/tracker.rb

@@ -9,7 +9,8 @@ require 'brakeman/processors/lib/find_all_calls'
 class Brakeman::Tracker
   attr_accessor :controllers, :templates, :models, :errors,
     :checks, :initializers, :config, :routes, :processor, :libs,
-    :template_cache, :options, :filter_cache
+    :template_cache, :options, :filter_cache, :start_time, :end_time,
+    :duration
 
   #Place holder when there should be a model, but it is not
   #clear what model it will be.
@@ -44,6 +45,9 @@ class Brakeman::Tracker
     @template_cache = Set.new
     @filter_cache = {}
     @call_index = nil
+    @start_time = Time.now
+    @end_time = nil
+    @duration = nil
   end
 
   #Add an error to the list. If no backtrace is given,
@@ -64,6 +68,10 @@ class Brakeman::Tracker
   #in Tracker#checks.
   def run_checks
     @checks = Brakeman::Checks.run_checks(self)
+
+    @end_time = Time.now
+    @duration = @end_time - @start_time
+    @checks
   end
 
   #Iterate over all methods in controllers and models.

lib/brakeman/warning.rb

@@ -76,14 +76,14 @@ class Brakeman::Warning
 
   #Return String of the code output from the OutputProcessor and
   #stripped of newlines and tabs.
-  def format_code
-    Brakeman::OutputProcessor.new.format(self.code).gsub(/(\t|\r|\n)+/, " ")
+  def format_code strip = true
+    format_ruby self.code, strip
   end
 
   #Return String of the user input formatted and
   #stripped of newlines and tabs.
-  def format_user_input
-    Brakeman::OutputProcessor.new.format(self.user_input).gsub(/(\t|\r|\n)+/, " ")
+  def format_user_input strip = true
+    format_ruby self.user_input, strip
   end
 
   #Return formatted warning message
@@ -171,9 +171,9 @@ class Brakeman::Warning
       :file => self.file,
       :line => self.line,
       :link => self.link,
-      :code => (@code && self.format_code),
+      :code => (@code && self.format_code(false)),
       :location => location,
-      :user_input => (@user_input && self.format_user_input),
+      :user_input => (@user_input && self.format_user_input(false)),
       :confidence => TEXT_CONFIDENCE[self.confidence]
     }
   end
@@ -181,4 +181,12 @@ class Brakeman::Warning
   def to_json
     MultiJson.dump self.to_hash
   end
+
+  private
+
+  def format_ruby code, strip
+    formatted = Brakeman::OutputProcessor.new.format(code)
+    formatted.gsub!(/(\t|\r|\n)+/, " ") if strip
+    formatted
+  end
 end

test/README.md

 ## Testing
 
-Run `ruby test.rb`.
+Run `rake` or if you want to avoid bundler `cd test && ruby test.rb`.
 
 This runs Brakeman against full apps in the `apps` directory and checks the results against what is expected.
 
 ## Test Generation
 
-Run `ruby to_test.rb apps/some_app > tests/test_some_app.rb` to generate a test suite with tests for each warning reported.
+Run `cd test && ruby to_test.rb apps/some_app > tests/test_some_app.rb` to generate a test suite with tests for each warning reported.

test/tests/test_json_compare.rb

@@ -9,6 +9,10 @@ class JSONCompareTests < Test::Unit::TestCase
     @report = MultiJson.load File.read(@json_path)
   end
 
+  def teardown
+    File.delete @json_path if File.exist? @json_path
+  end
+
   def update_json
     File.open @json_path, "w" do |f|
       f.puts @report.to_json