First pass at annotations / ignore "known false-positives"

README.md

@@ -115,6 +115,32 @@ To compare results of a scan with a previous scan, use the JSON output option an
 
 This will output JSON with two lists: one of fixed warnings and one of new warnings.
 
+## Using annotations to ignore false positives
+
+Brakeman can now produce an "annotation" output format via `-f annotation`.  The output of this format is a YAML file which marks up the various warnings produced in a brakeman run.  The intention of this format is to extract the various warnings your security team has identified as technically a vulnerability but one that will not affect the system's integrity or that the service owner has accepted risk on (for you ITIL types).  The general workflow for using annotations is as follows:
+
+1. Run brakeman with `-f annotation -o brakeman_annotations.yaml` options.
+2. Extract the warnings you wish to ignore as false-positives from `brakeman_annotations.yaml` into `.brakeman_annotations.yaml` noting the differing leading periods.  The `-A` flag defaults to using the `.brakeman_annotations.yaml` filename but another could just as easily be used.  Also make sure the leading three dashes (part of the YAML markup) remain in place.  Do not rearrange the lines of the YAML encoded hash before the `hash` attribute but you may edit everything occurring after the `hash` attribute.  For instance you might edit the note attribute to indicate why you have chosen to ignore that mass-assignment warning we've created for you.
+
+```yaml
+    ---
+    - :warning_type: Mass Assignment
+      :message: Unprotected mass assignment
+      :file: /Users/mrdev/Documents/my/app/controllers/vulernablity_controller.rb
+      :code: Vulnerability.new(params[:vulnerability])
+      :location:
+        :type: :method
+        :class: :VulnerabilityController
+        :method: :not_create
+      :confidence: High
+      :line: 51
+      :hash: 29d2d5ec2b388060c746d6901e477ef9
+      :note: 'We ignore this because this action does not have a route'
+```
+3. Run Brakeman with the `-A <filename>` option where the filename is the name of your new annotations file if you haven't used the default name.  The warnings from your annotation file should be ignored with a note to that effect in the report.
+
+For best results use the `--separate-models` option to brakeman.  That way you can ignore a single mass-assignment vulnerability (for example) and not ignore _all_ mass-assignments vulnerabilities which would be The Wrong Thing :TM:
+
 # Warning information
 
 See WARNING\_TYPES for more information on the warnings reported by this tool.

lib/brakeman.rb

@@ -134,20 +134,22 @@ def self.get_output_formats options
     end
     if options[:output_format]
       [
-        case options[:output_format]
-        when :html, :to_html
-          :to_html
-        when :csv, :to_csv
-          :to_csv
-        when :pdf, :to_pdf
-          :to_pdf
-        when :tabs, :to_tabs
-          :to_tabs
-        when :json, :to_json
-          :to_json
-        else
-          :to_s
-        end
+      case options[:output_format]
+      when :html, :to_html
+        :to_html
+      when :csv, :to_csv
+        :to_csv
+      when :pdf, :to_pdf
+        :to_pdf
+      when :tabs, :to_tabs
+        :to_tabs
+      when :json, :to_json
+        :to_json
+      when :annotation, :to_annotation
+        :to_annotation
+      else
+        :to_s
+      end
       ]
     else
       return [:to_s] unless options[:output_files]
@@ -163,6 +165,8 @@ def self.get_output_formats options
           :to_tabs
         when /\.json$/i
           :to_json
+        when /\.annotations/
+          :to_annotation
         else
           :to_s
         end
@@ -262,20 +266,23 @@ def self.scan options
     end
     tracker.run_checks
 
+    reporter = tracker.report
+    reporter.filter_by_annotations(options[:annotations_file]) if options[:annotations_file]
+
     if options[:output_files]
       notify "Generating report..."
 
       options[:output_files].each_with_index do |output_file, idx|
         File.open output_file, "w" do |f|
-          f.write tracker.report.send(options[:output_formats][idx])
+          f.write reporter.send(options[:output_formats][idx])
         end
         notify "Report saved in '#{output_file}'"
       end
     elsif options[:print_report]
       notify "Generating report..."
 
       options[:output_formats].each do |output_format|
-        puts tracker.report.send(output_format)
+        puts reporter.send(output_format)
       end
     end
 

lib/brakeman/checks.rb

@@ -1,5 +1,6 @@
 require 'thread'
 require 'brakeman/differ'
+require 'set'
 
 #Collects up results from running different checks.
 #
@@ -9,7 +10,7 @@
 class Brakeman::Checks
   @checks = []
 
-  attr_reader :warnings, :controller_warnings, :model_warnings, :template_warnings, :checks_run
+  attr_reader :warnings, :controller_warnings, :model_warnings, :template_warnings, :ignored_warnings, :checks_run
 
   #Add a check. This will call +_klass_.new+ when running tests
   def self.add klass
@@ -32,6 +33,7 @@ def initialize options = { }
     @template_warnings = []
     @model_warnings = []
     @controller_warnings = []
+    @ignored_warnings = []
     @checks_run = []
   end
 
@@ -57,6 +59,15 @@ def add_warning warning
     end
   end
 
+  def filter_by_annotations(annotations)
+    ignored_warning_digests = Set.new(annotations.map { |a| a[:digest] })
+    @ignored_warnings = all_warnings.select { |w| ignored_warning_digests.include? w.annotation_digest }
+
+    [@warnings, @template_warnings, @controller_warnings, @model_warnings].each do |warning_group|
+      warning_group.reject! { |w| ignored_warning_digests.include? w.annotation_digest }
+    end
+  end
+
   #Return a hash of arrays of new and fixed warnings
   #
   #    diff = checks.diff old_checks

lib/brakeman/options.rb

@@ -129,7 +129,7 @@ def get_options args, destructive = false
 
         opts.on "-f", 
           "--format TYPE", 
-          [:pdf, :text, :html, :csv, :tabs, :json],
+          [:pdf, :text, :html, :csv, :tabs, :json, :annotations],
           "Specify output formats. Default is text" do |type|
 
           type = "s" if type == :text
@@ -161,6 +161,10 @@ def get_options args, destructive = false
           options[:output_files].push(file)
         end
 
+        opts.on "-A", "--use-annotations FILE", "Use annotations file" do |file|
+          options[:annotations_file] = file
+        end
+
         opts.on "--separate-models", "Warn on each model without attr_accessible" do
           options[:collapse_mass_assignment] = false
         end

lib/brakeman/report.rb

@@ -35,11 +35,13 @@ def initialize tracker
     @element_id = 0 #Used for HTML ids
     @warnings_summary = nil
     @highlight_user_input = tracker.options[:highlight_user_input]
+    @ignored_summary = nil
   end
 
   #Generate summary table of what was parsed
   def generate_overview html = false
     warnings = all_warnings.length
+    ignored_warnings = checks.ignored_warnings
 
     if html
       load_and_render_erb('overview', binding)
@@ -50,13 +52,14 @@ def generate_overview html = false
         t.add_row ['Templates', number_of_templates(@tracker)]
         t.add_row ['Errors', tracker.errors.length]
         t.add_row ['Security Warnings', "#{warnings} (#{warnings_summary[:high_confidence]})"]
+        t.add_row ['Ignored Warnings', ignored_warnings.length]
       end
     end
   end
 
   #Generate table of how many warnings of each warning type were reported
   def generate_warning_overview html = false
-    types = warnings_summary.keys
+    types = warnings_summary.keys | ignored_summary.keys
     types.delete :high_confidence
 
     if html
@@ -316,6 +319,31 @@ def generate_templates html = false
     end
   end
 
+  def generate_ignored_notes html = false
+    if checks.ignored_warnings
+      warnings = []
+      checks.ignored_warnings.each do |warning|
+        annotation =  @annotations.find{|a| a[:digest] == warning.annotation_digest}
+        if annotation[:note] && annotation[:note].length > 0
+          warnings << {"Warning" => "#{warning.warning_type} (#{File.basename(warning.file)}:#{warning.line})",
+                       "Note" => annotation[:note] }
+        end
+      end
+
+      if warnings.length > 0
+        if html
+          load_and_render_erb("ignored_notes", binding)
+        else
+          Terminal::Table.new(:headings => ['Warning', 'Note']) do |t|
+            warnings.each do |warning|
+              t.add_row [warning["Warning"], warning["Note"]]
+            end
+          end
+        end
+      end
+    end
+  end
+
   #Generate HTML output
   def to_html
     out = html_header <<
@@ -340,6 +368,7 @@ def to_html
     out << generate_controller_warnings(true).to_s
     out << generate_model_warnings(true).to_s
     out << generate_template_warnings(true).to_s
+    out << generate_ignored_notes(true).to_s
 
     out << "</body></html>"
   end
@@ -381,6 +410,9 @@ def to_s
     res = generate_template_warnings
     out << "\n\nView Warnings:\n\n" << truncate_table(res.to_s) if res
 
+    res = generate_ignored_notes
+    out << "\n\nIgnored Warnings Notes:\n\n" << truncate_table(res.to_s) if res
+
     out << "\n"
     out
   end
@@ -490,6 +522,15 @@ def warnings_summary
     @warnings_summary = summary
   end
 
+  def ignored_summary
+    return @ignored_summary if @ignored_summary
+    summary = Hash.new(0)
+    checks.ignored_warnings.each do |warning|
+      summary[warning.warning_type.to_s] += 1
+    end
+    @ignored_summary = summary
+  end
+
   #Escape warning message and highlight user input in text output
   def text_message warning, message
     if @highlight_user_input and warning.user_input
@@ -545,7 +586,7 @@ def with_context warning, message
       message
     end <<
     "<table id='#{code_id}' class='context' style='display:none'>" <<
-    "<caption>#{(warning.file || '').gsub(tracker.options[:app_path], "")}</caption>"
+    "<caption>#{warning_file_to_relative_path(warning.file)}</caption>"
 
     unless context.empty?
       if warning.line - 1 == 1 or warning.line + 1 == 1
@@ -668,11 +709,39 @@ def number_of_templates tracker
     Set.new(tracker.templates.map {|k,v| v[:name].to_s[/[^.]+/]}).length
   end
 
+  def filter_by_annotations annotations_file
+    load_annotations annotations_file
+    @checks.filter_by_annotations(@annotations)
+  end
+
   private
 
   def load_and_render_erb file, bind
     content = File.read(File.expand_path("templates/#{file}.html.erb", File.dirname(__FILE__)))
     template = ERB.new(content)
     template.result(bind)
   end
+
+  def to_annotation
+    warnings = @checks.all_warnings
+    warnings.map do |warning|
+      warning.file = warning_file_to_relative_path(warning.file)
+      warning.to_annotation
+    end.to_yaml
+  end
+
+  def load_annotations annotations_file
+    annotations_file ||= ""
+
+    [File.expand_path(annotations_file), File.expand_path("./.brakeman_annotations.yaml")].each do |f|
+      if File.exist? f and not File.directory? f
+        notify "[Notice] Using annotations in #{f}"
+        @annotations = YAML::load_file f
+      end
+    end
+  end
+
+  def warning_file_to_relative_path(warning_file)
+    (warning_file || '').gsub(tracker.options[:app_path], "").gsub(/^\//, '')
+  end
 end

lib/brakeman/templates/ignored_notes.html.erb

@@ -0,0 +1,13 @@
+<p>Ignored Warnings Notes</p>
+<table>
+  <tr>
+    <th>Warning</th>
+    <th>Note</th>
+  </tr>
+<% warnings.each do |warning| %>
+  <tr>
+    <td><%= warning["Warning"] %></td>
+    <td><%= warning["Note"] %></td>
+  </tr>
+<% end %>
+</table>

lib/brakeman/warning.rb

@@ -1,3 +1,5 @@
+require 'digest/md5'
+
 #The Warning class stores information about warnings
 class Brakeman::Warning
   attr_reader :called_from, :check, :class, :confidence, :controller,
@@ -163,4 +165,28 @@ def to_json
 
     JSON.dump self.to_hash
   end
+
+  def to_annotation
+    self.to_hash.merge({:digest => self.annotation_digest, :note => ""})
+  end
+
+  def clean_for_annotation
+    h = self.to_hash
+    h.delete(:line)
+    h
+  end
+
+  def annotation_digest
+    digested = ""
+    clean_annotation = clean_for_annotation
+    clean_annotation.keys.map(&:to_s).sort.each do |k|
+      digested << k << clean_annotation[k.to_sym].to_s
+    end
+
+    digest = Digest::MD5.hexdigest(digested)
+    if RUBY_VERSION >= "1.9"
+      digest.force_encoding("UTF-8")
+    end
+    digest
+  end
 end