diff --git a/lib/brakeman.rb b/lib/brakeman.rb index f1f8ca25d7be4bb473301f45882d2698c7e26eba..4a8e17bd7f5f6c098dc99ecd27332014b2b60889 100644 --- a/lib/brakeman.rb +++ b/lib/brakeman.rb @@ -40,7 +40,7 @@ module Brakeman # * :skip_libs - do not process lib/ directory (default: false) # * :skip_checks - checks not to run (run all if not specified) # * :relative_path - show relative path of each file(default: false) - # * :summary_only - only output summary section of report + # * :summary_only - only output summary section of report # (does not apply to tabs format) # #Alternatively, just supply a path as a string. @@ -68,15 +68,6 @@ module Brakeman options = get_defaults.merge! options options[:output_formats] = get_output_formats options - app_path = options[:app_path] - - abort("Please supply the path to a Rails application.") unless app_path and File.exist? app_path + "/app" - - if File.exist? app_path + "/script/rails" - options[:rails3] = true - notify "[Notice] Detected Rails 3 application" unless options[:quiet] - end - options end @@ -116,8 +107,8 @@ module Brakeman #Default set of options def self.get_defaults - { :skip_checks => Set.new, - :check_arguments => true, + { :skip_checks => Set.new, + :check_arguments => true, :safe_methods => Set.new, :min_confidence => 2, :combine_locations => true, @@ -130,7 +121,7 @@ module Brakeman :relative_path => false, :quiet => true, :report_progress => true, - :html_style => "#{File.expand_path(File.dirname(__FILE__))}/brakeman/format/style.css" + :html_style => "#{File.expand_path(File.dirname(__FILE__))}/brakeman/format/style.css" } end diff --git a/lib/brakeman/app_tree.rb b/lib/brakeman/app_tree.rb new file mode 100644 index 0000000000000000000000000000000000000000..f629cd460cf36513c2f9f96ed40a035ca0f6861c --- /dev/null +++ b/lib/brakeman/app_tree.rb @@ -0,0 +1,90 @@ +module Brakeman + class AppTree + VIEW_EXTENSIONS = %w[html.erb html.haml rhtml js.erb].join(",") + + attr_reader :root + + def self.from_options(options) + root = options[:app_path] + + # Convert files into Regexp for matching + if options[:skip_files] + list = "(?:" << options[:skip_files].map { |f| Regexp.escape f }.join("|") << ")$" + new(root, Regexp.new(list)) + else + new(root) + end + end + + def initialize(root, skip_files = nil) + @root = root + @skip_files = skip_files + end + + def expand_path(path) + File.expand_path(path, @root) + end + + def read(path) + File.read(File.join(@root, path)) + end + + # This variation requires full paths instead of paths based + # off the project root. I'd prefer to get all the code outside + # of AppTree using project-root based paths (e.g. app/models/user.rb) + # instead of full paths, but I suspect it's an incompatible change. + def read_path(path) + File.read(path) + end + + def exists?(path) + File.exists?(File.join(@root, path)) + end + + # This is a pair for #read_path. Again, would like to kill these + def path_exists?(path) + File.exists?(path) + end + + def initializer_paths + @initializer_paths ||= find_paths("config/initializers") + end + + def controller_paths + @controller_paths ||= find_paths("app/controllers") + end + + def model_paths + @model_paths ||= find_paths("app/models") + end + + def template_paths + @template_paths ||= find_paths("app/views", "*.{#{VIEW_EXTENSIONS}}") + end + + def layout_exists?(name) + pattern = "#{@root}/app/views/layouts/#{name}.html.{erb,haml}" + !Dir.glob(pattern).empty? + end + + def lib_paths + @lib_files ||= find_paths("lib") + end + + private + + def find_paths(directory, extensions = "*.rb") + pattern = @root + "/#{directory}/**/#{extensions}" + + Dir.glob(pattern).sort.tap do |paths| + reject_skipped_files(paths) + end + end + + def reject_skipped_files(paths) + return unless @skip_files + paths.reject! { |f| @skip_files.match f } + end + + end +end diff --git a/lib/brakeman/checks.rb b/lib/brakeman/checks.rb index 3d30b80c9b0a56baca66264d33591ca2127722ec..2ead86a87303cbea6de51e9e031f8ee6bfa587da 100644 --- a/lib/brakeman/checks.rb +++ b/lib/brakeman/checks.rb @@ -75,28 +75,28 @@ class Brakeman::Checks #Run all the checks on the given Tracker. #Returns a new instance of Checks with the results. - def self.run_checks tracker + def self.run_checks(app_tree, tracker) if tracker.options[:parallel_checks] - self.run_checks_parallel tracker + self.run_checks_parallel(app_tree, tracker) else - self.run_checks_sequential tracker + self.run_checks_sequential(app_tree, tracker) end end #Run checks sequentially - def self.run_checks_sequential tracker + def self.run_checks_sequential(app_tree, tracker) check_runner = self.new :min_confidence => tracker.options[:min_confidence] @checks.each do |c| check_name = get_check_name c #Run or don't run check based on options - unless tracker.options[:skip_checks].include? check_name or + unless tracker.options[:skip_checks].include? check_name or (tracker.options[:run_checks] and not tracker.options[:run_checks].include? check_name) Brakeman.notify " - #{check_name}" - check = c.new(tracker) + check = c.new(app_tree, tracker) begin check.run_check @@ -118,23 +118,23 @@ class Brakeman::Checks end #Run checks in parallel threads - def self.run_checks_parallel tracker + def self.run_checks_parallel(app_tree, tracker) threads = [] error_mutex = Mutex.new - + check_runner = self.new :min_confidence => tracker.options[:min_confidence] @checks.each do |c| check_name = get_check_name c #Run or don't run check based on options - unless tracker.options[:skip_checks].include? check_name or + unless tracker.options[:skip_checks].include? check_name or (tracker.options[:run_checks] and not tracker.options[:run_checks].include? check_name) Brakeman.notify " - #{check_name}" threads << Thread.new do - check = c.new(tracker) + check = c.new(app_tree, tracker) begin check.run_check @@ -175,6 +175,6 @@ class Brakeman::Checks end #Load all files in checks/ directory -Dir.glob("#{File.expand_path(File.dirname(__FILE__))}/checks/*.rb").sort.each do |f| +Dir.glob("#{File.expand_path(File.dirname(__FILE__))}/checks/*.rb").sort.each do |f| require f.match(/(brakeman\/checks\/.*)\.rb$/)[0] end diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index fa8724307555dea77d33c50471936edff82fe178..79f4cbc6fda9541a1e7115e1d71ab24a9e8317d4 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -14,8 +14,9 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor Match = Struct.new(:type, :match) #Initialize Check with Checks. - def initialize tracker + def initialize(app_tree, tracker) super() + @app_tree = app_tree @results = [] #only to check for duplicates @warnings = [] @tracker = tracker @@ -105,13 +106,13 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor private - #Report a warning + #Report a warning def warn options warning = Brakeman::Warning.new(options.merge({ :check => self.class.to_s })) warning.file = file_for warning - @warnings << warning - end + @warnings << warning + end #Run _exp_ through OutputProcessor to get a nice String. def format_output exp @@ -149,12 +150,12 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor #Checks if mass assignment is disabled globally in an initializer. def mass_assign_disabled? return @mass_assign_disabled unless @mass_assign_disabled.nil? - + @mass_assign_disabled = false - if version_between?("3.1.0", "4.0.0") and + if version_between?("3.1.0", "4.0.0") and tracker.config[:rails] and - tracker.config[:rails][:active_record] and + tracker.config[:rails][:active_record] and tracker.config[:rails][:active_record][:whitelist_attributes] == Sexp.new(:true) @mass_assign_disabled = true @@ -367,7 +368,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor #Checks if +exp+ is a model name. # - #Prior to using this method, either @tracker must be set to + #Prior to using this method, either @tracker must be set to #the current tracker, or else @models should contain an array of the model #names, which is available via tracker.models.keys def model_name? exp @@ -390,14 +391,14 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor #Finds entire method call chain where +target+ is a target in the chain def find_chain exp, target - return unless sexp? exp + return unless sexp? exp case exp.node_type when :output, :format find_chain exp.value, target when :call if exp == target or include_target? exp, target - return exp + return exp end else exp.each do |e| @@ -451,7 +452,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor end def gemfile_or_environment - if File.exist? File.expand_path "#{tracker.options[:app_path]}/Gemfile" + if @app_tree.exists?("Gemfile") "Gemfile" else "config/environment.rb" diff --git a/lib/brakeman/processor.rb b/lib/brakeman/processor.rb index 5e48a8f781b66386844c05e59799b80d62eb1dd2..cf0748a0cb1499a9d9d1ce78ec41c6b8847f7111 100644 --- a/lib/brakeman/processor.rb +++ b/lib/brakeman/processor.rb @@ -12,8 +12,9 @@ module Brakeman class Processor include Util - def initialize options - @tracker = Tracker.new self, options + def initialize(app_tree, options) + @app_tree = app_tree + @tracker = Tracker.new(@app_tree, self, options) end def tracked_events @@ -38,7 +39,7 @@ module Brakeman #Process controller source. +file_name+ is used for reporting def process_controller src, file_name if contains_class? src - ControllerProcessor.new(@tracker).process_controller src, file_name + ControllerProcessor.new(@app_tree, @tracker).process_controller src, file_name else LibraryProcessor.new(@tracker).process_library src, file_name end @@ -47,7 +48,7 @@ module Brakeman #Process variable aliasing in controller source and save it in the #tracker. def process_controller_alias name, src, only_method = nil - ControllerAliasProcessor.new(@tracker, only_method).process_controller name, src + ControllerAliasProcessor.new(@app_tree, @tracker, only_method).process_controller name, src end #Process a model source diff --git a/lib/brakeman/processors/controller_alias_processor.rb b/lib/brakeman/processors/controller_alias_processor.rb index 9f160f8c7938bfe9f15c3c8280855d640e7be2cd..701f9351d5a52cd9ed7e2f9b13a16667b64c65f2 100644 --- a/lib/brakeman/processors/controller_alias_processor.rb +++ b/lib/brakeman/processors/controller_alias_processor.rb @@ -9,8 +9,9 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor #If only_method is specified, only that method will be processed, #other methods will be skipped. #This is for rescanning just a single action. - def initialize tracker, only_method = nil + def initialize app_tree, tracker, only_method = nil super() + @app_tree = app_tree @only_method = only_method @tracker = tracker @rendered = false @@ -46,7 +47,7 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor methods.each do |name| #Need to process the method like it was in a controller in order #to get the renders set - processor = Brakeman::ControllerProcessor.new(@tracker) + processor = Brakeman::ControllerProcessor.new(@app_tree, @tracker) method = mixin[:public][name] if node_type? method, :methdef @@ -132,7 +133,7 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor #Processes a call to a before filter. #Basically, adds any instance variable assignments to the environment. #TODO: method arguments? - def process_before_filter name + def process_before_filter name filter = find_method name, @current_class if filter.nil? @@ -236,9 +237,9 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor end controller[:before_filter_cache].each do |f| - if f[:all] or + if f[:all] or (f[:only] == method) or - (f[:only].is_a? Array and f[:only].include? method) or + (f[:only].is_a? Array and f[:only].include? method) or (f[:except].is_a? Symbol and f[:except] != method) or (f[:except].is_a? Array and not f[:except].include? method) diff --git a/lib/brakeman/processors/controller_processor.rb b/lib/brakeman/processors/controller_processor.rb index 27c940323426b730721164b7429e8a1144c16399..4964642bb745d68fca16d2a5db8200d21f0a2eb1 100644 --- a/lib/brakeman/processors/controller_processor.rb +++ b/lib/brakeman/processors/controller_processor.rb @@ -4,8 +4,9 @@ require 'brakeman/processors/base_processor' class Brakeman::ControllerProcessor < Brakeman::BaseProcessor FORMAT_HTML = Sexp.new(:call, Sexp.new(:lvar, :format), :html) - def initialize tracker - super + def initialize app_tree, tracker + super(tracker) + @app_tree = app_tree @controller = nil @current_method = nil @current_module = nil @@ -89,7 +90,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor #layout "some_layout" name = args.last.value.to_s - unless Dir.glob("#{@tracker.options[:app_path]}/app/views/layouts/#{name}.html.{erb,haml}").empty? + if @app_tree.layout_exists?(name) @controller[:layout] = "layouts/#{name}" else Brakeman.debug "[Notice] Layout not found: #{name}" @@ -107,7 +108,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor exp elsif target == nil and method == :render make_render exp - elsif exp == FORMAT_HTML and context[1] != :iter + elsif exp == FORMAT_HTML and context[1] != :iter #This is an empty call to # format.html #Which renders the default template if no arguments @@ -174,7 +175,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor name = underscore(@controller[:name].to_s.split("::")[-1].gsub("Controller", '')) #There is a layout for this Controller - unless Dir.glob("#{@tracker.options[:app_path]}/app/views/layouts/#{name}.html.{erb,haml}").empty? + if @app_tree.layout_exists?(name) @controller[:layout] = "layouts/#{name}" end end diff --git a/lib/brakeman/report.rb b/lib/brakeman/report.rb index 310c2b603c4e5deca60d68b8f85ce6f883ae47fd..166fe0357c0ca2999ca2f631afa65e7c56c436de 100644 --- a/lib/brakeman/report.rb +++ b/lib/brakeman/report.rb @@ -40,7 +40,8 @@ class Brakeman::Report "Medium", "Weak" ] - def initialize tracker + def initialize(app_tree, tracker) + @app_tree = app_tree @tracker = tracker @checks = tracker.checks @element_id = 0 #Used for HTML ids @@ -158,7 +159,7 @@ class Brakeman::Report end return nil if warnings.empty? - + stabilizer = 0 warnings = warnings.sort_by{|row| stabilizer += 1; [row["Confidence"], row["Warning Type"], row["Template"], stabilizer]} if html @@ -232,7 +233,7 @@ class Brakeman::Report end return nil if warnings.empty? - + stabilizer = 0 warnings = warnings.sort_by{|row| stabilizer +=1; [row["Confidence"], row["Warning Type"], row["Controller"], stabilizer]} @@ -318,7 +319,7 @@ class Brakeman::Report else output = '' template_rows.each do |template| - output << template.first.to_s << "\n\n" + output << template.first.to_s << "\n\n" table = Terminal::Table.new(:headings => ['Output']) do |t| # template[1] is an array of calls template[1].each do |v| @@ -392,7 +393,7 @@ class Brakeman::Report res = generate_controller_warnings out << "\n\n\nController Warnings:\n\n" << truncate_table(res.to_s) if res - res = generate_model_warnings + res = generate_model_warnings out << "\n\n\nModel Warnings:\n\n" << truncate_table(res.to_s) if res res = generate_template_warnings @@ -404,8 +405,8 @@ class Brakeman::Report #Generate CSV output def to_csv - output = csv_header - output << "\nSUMMARY\n" + output = csv_header + output << "\nSUMMARY\n" output << table_to_csv(generate_overview) << "\n" @@ -437,7 +438,7 @@ class Brakeman::Report output << table_to_csv(res) << "\n" if res output << "Model Warnings\n" - res = generate_model_warnings + res = generate_model_warnings output << table_to_csv(res) << "\n" if res res = generate_template_warnings @@ -542,11 +543,11 @@ HEADER #Generate HTML for warnings, including context show/hidden via Javascript def with_context warning, message - context = context_for warning + context = context_for(@app_tree, warning) full_message = nil if tracker.options[:message_limit] and - tracker.options[:message_limit] > 0 and + tracker.options[:message_limit] > 0 and message.length > tracker.options[:message_limit] full_message = html_message(warning, message) @@ -657,12 +658,12 @@ HEADER else w.code = "" end - w.context = context_for(w).join("\n") + w.context = context_for(@app_tree, w).join("\n") end end report[:config] = tracker.config - + report end diff --git a/lib/brakeman/rescanner.rb b/lib/brakeman/rescanner.rb index 3a4b8f90cf8429b1114f6d44ea6bb8ff28dcdd14..e87d19b6b7f4b042bcca8c4099362a60f71c196f 100644 --- a/lib/brakeman/rescanner.rb +++ b/lib/brakeman/rescanner.rb @@ -12,7 +12,7 @@ class Brakeman::Rescanner < Brakeman::Scanner def initialize options, processor, changed_files super(options, processor) - @paths = changed_files.map {|f| File.expand_path f, tracker.options[:app_path] } + @paths = changed_files.map {|f| @app_tree.expand_path(f) } @old_results = tracker.checks #Old warnings from previous scan @changes = nil #True if files had to be rescanned @reindex = Set.new @@ -66,7 +66,7 @@ class Brakeman::Rescanner < Brakeman::Scanner def rescan_file path, type = nil type ||= file_type path - unless File.exist? path + unless @app_tree.path_exists?(path) return rescan_deleted_file path, type end @@ -128,7 +128,7 @@ class Brakeman::Rescanner < Brakeman::Scanner end def rescan_template path - return unless path.match KNOWN_TEMPLATE_EXTENSIONS and File.exist? path + return unless path.match KNOWN_TEMPLATE_EXTENSIONS and @app_tree.path_exists?(path) template_name = template_path_to_name(path) @@ -177,7 +177,7 @@ class Brakeman::Rescanner < Brakeman::Scanner def rescan_model path num_models = tracker.models.length tracker.reset_model path - process_model path if File.exists? path + process_model path if @app_tree.path_exists?(path) #Only need to rescan other things if a model is added or removed if num_models != tracker.models.length @@ -190,7 +190,7 @@ class Brakeman::Rescanner < Brakeman::Scanner end def rescan_lib path - process_lib path if File.exists? path + process_lib path if @app_tree.path_exists?(path) lib = nil diff --git a/lib/brakeman/scanner.rb b/lib/brakeman/scanner.rb index 2759253773e0852794422dbb840e53bc88d17680..3b25939b7c13b65012681d4df4bee08bf9e9324a 100644 --- a/lib/brakeman/scanner.rb +++ b/lib/brakeman/scanner.rb @@ -9,6 +9,7 @@ begin require 'erb' require 'erubis' require 'brakeman/processor' + require 'brakeman/app_tree' require 'brakeman/parsers/rails2_erubis' require 'brakeman/parsers/rails2_xss_plugin_erubis' require 'brakeman/parsers/rails3_erubis' @@ -28,19 +29,19 @@ class Brakeman::Scanner #Pass in path to the root of the Rails application def initialize options, processor = nil @options = options - @report_progress = options[:report_progress] - @path = options[:app_path] - @app_path = File.join(@path, "app") - @processor = processor || Brakeman::Processor.new(options) - @skip_files = nil - - #Convert files into Regexp for matching - if options[:skip_files] - list = "(?:" << options[:skip_files].map { |f| Regexp.escape f }.join("|") << ")$" - @skip_files = Regexp.new(list) + @app_tree = Brakeman::AppTree.from_options(options) + + if !@app_tree.root || !@app_tree.exists?("app") + abort("Please supply the path to a Rails application.") + end + + if @app_tree.exists?("script/rails") + options[:rails3] = true + Brakeman.notify "[Notice] Detected Rails 3 application" end @ruby_parser = ::RubyParser + @processor = processor || Brakeman::Processor.new(@app_tree, options) end #Returns the Tracker generated from the scan @@ -83,7 +84,7 @@ class Brakeman::Scanner process_config_file "gems.rb" end - if File.exists? "#@path/vendor/plugins/rails_xss" or + if @app_tree.exists?("vendor/plugins/rails_xss") or options[:rails3] or options[:escape_html] tracker.config[:escape_html] = true @@ -92,8 +93,10 @@ class Brakeman::Scanner end def process_config_file file - if File.exists? "#@path/config/#{file}" - @processor.process_config(parse_ruby(File.read("#@path/config/#{file}"))) + path = "config/#{file}" + + if @app_tree.exists?(path) + @processor.process_config(parse_ruby(@app_tree.read(path))) end rescue Exception => e @@ -105,11 +108,11 @@ class Brakeman::Scanner #Process Gemfile def process_gems - if File.exists? "#@path/Gemfile" - if File.exists? "#@path/Gemfile.lock" - @processor.process_gems(parse_ruby(File.read("#@path/Gemfile")), File.read("#@path/Gemfile.lock")) + if @app_tree.exists? "Gemfile" + if @app_tree.exists? "Gemfile.lock" + @processor.process_gems(parse_ruby(@app_tree.read("Gemfile")), @app_tree.read("Gemfile.lock")) else - @processor.process_gems(parse_ruby(File.read("#@path/Gemfile"))) + @processor.process_gems(parse_ruby(@app_tree.read("Gemfile"))) end end rescue Exception => e @@ -121,10 +124,7 @@ class Brakeman::Scanner # #Adds parsed information to tracker.initializers def process_initializers - initializer_files = Dir.glob(@path + "/config/initializers/**/*.rb").sort - initializer_files.reject! { |f| @skip_files.match f } if @skip_files - - initializer_files.each do |f| + @app_tree.initializer_paths.each do |f| process_initializer f end end @@ -132,7 +132,7 @@ class Brakeman::Scanner #Process an initializer def process_initializer path begin - @processor.process_initializer(path, parse_ruby(File.read(path))) + @processor.process_initializer(path, parse_ruby(@app_tree.read_path(path))) rescue Racc::ParseError => e tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'" rescue Exception => e @@ -149,19 +149,13 @@ class Brakeman::Scanner return end - lib_files = Dir.glob(@path + "/lib/**/*.rb").sort - lib_files.reject! { |f| @skip_files.match f } if @skip_files - - total = lib_files.length + total = @app_tree.lib_paths.length current = 0 - lib_files.each do |f| + @app_tree.lib_paths.each do |f| Brakeman.debug "Processing #{f}" - if @report_progress - $stderr.print " #{current}/#{total} files processed\r" - current += 1 - end - + report_progress(current, total) + current += 1 process_lib f end end @@ -169,7 +163,7 @@ class Brakeman::Scanner #Process a library def process_lib path begin - @processor.process_lib parse_ruby(File.read(path)), path + @processor.process_lib parse_ruby(@app_tree.read_path(path)), path rescue Racc::ParseError => e tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'" rescue Exception => e @@ -181,9 +175,9 @@ class Brakeman::Scanner # #Adds parsed information to tracker.routes def process_routes - if File.exists? "#@path/config/routes.rb" + if @app_tree.exists?("config/routes.rb") begin - @processor.process_routes parse_ruby(File.read("#@path/config/routes.rb")) + @processor.process_routes parse_ruby(@app_tree.read("config/routes.rb")) rescue Exception => e tracker.error e.exception(e.message + "\nWhile processing routes.rb"), e.backtrace Brakeman.notify "[Notice] Error while processing routes - assuming all public controller methods are actions." @@ -198,19 +192,13 @@ class Brakeman::Scanner # #Adds processed controllers to tracker.controllers def process_controllers - controller_files = Dir.glob(@app_path + "/controllers/**/*.rb").sort - controller_files.reject! { |f| @skip_files.match f } if @skip_files - - total = controller_files.length + total = @app_tree.controller_paths.length current = 0 - controller_files.each do |f| + @app_tree.controller_paths.each do |f| Brakeman.debug "Processing #{f}" - if @report_progress - $stderr.print " #{current}/#{total} files processed\r" - current += 1 - end - + report_progress(current, total) + current += 1 process_controller f end @@ -221,11 +209,8 @@ class Brakeman::Scanner tracker.controllers.sort_by{|name| name.to_s}.each do |name, controller| Brakeman.debug "Processing #{name}" - if @report_progress - $stderr.print " #{current}/#{total} controllers processed\r" - current += 1 - end - + report_progress(current, total, "controllers") + current += 1 @processor.process_controller_alias name, controller[:src] end @@ -235,7 +220,7 @@ class Brakeman::Scanner def process_controller path begin - @processor.process_controller(parse_ruby(File.read(path)), path) + @processor.process_controller(parse_ruby(@app_tree.read_path(path)), path) rescue Racc::ParseError => e tracker.error e, "could not parse #{path}. There is probably a typo in the file. Test it with 'ruby_parse #{path}'" rescue Exception => e @@ -247,23 +232,15 @@ class Brakeman::Scanner # #Adds processed views to tracker.views def process_templates - - views_path = @app_path + "/views/**/*.{html.erb,html.haml,rhtml,js.erb}" $stdout.sync = true - count = 0 - - template_files = Dir.glob(views_path).sort - template_files.reject! { |f| @skip_files.match f } if @skip_files - total = template_files.length + count = 0 + total = @app_tree.template_paths.length - template_files.each do |path| + @app_tree.template_paths.each do |path| Brakeman.debug "Processing #{path}" - if @report_progress - $stderr.print " #{count}/#{total} files processed\r" - count += 1 - end - + report_progress(count, total) + count += 1 process_template path end @@ -274,11 +251,8 @@ class Brakeman::Scanner tracker.templates.keys.dup.sort_by{|name| name.to_s}.each do |name| Brakeman.debug "Processing #{name}" - if @report_progress - count += 1 - $stderr.print " #{count}/#{total} templates processed\r" - end - + report_progress(count, total, "templates") + count += 1 @processor.process_template_alias tracker.templates[name] end end @@ -287,7 +261,7 @@ class Brakeman::Scanner type = path.match(KNOWN_TEMPLATE_EXTENSIONS)[1].to_sym type = :erb if type == :rhtml name = template_path_to_name path - text = File.read path + text = @app_tree.read_path path begin if type == :erb @@ -339,27 +313,20 @@ class Brakeman::Scanner # #Adds the processed models to tracker.models def process_models - model_files = Dir.glob(@app_path + "/models/**/*.rb").sort - model_files.reject! { |f| @skip_files.match f } if @skip_files - - total = model_files.length + total = @app_tree.model_paths.length current = 0 - model_files.each do |f| + @app_tree.model_paths.each do |f| Brakeman.debug "Processing #{f}" - if @report_progress - $stderr.print " #{current}/#{total} files processed\r" - current += 1 - end - + report_progress(current, total) + current += 1 process_model f - end end def process_model path begin - @processor.process_model(parse_ruby(File.read(path)), path) + @processor.process_model(parse_ruby(@app_tree.read_path(path)), path) rescue Racc::ParseError => e tracker.error e, "could not parse #{path}" rescue Exception => e @@ -367,6 +334,11 @@ class Brakeman::Scanner end end + def report_progress(current, total, type = "files") + return unless @options[:report_progress] + $stderr.print " #{current}/#{total} #{type} processed\r" + end + def index_call_sites tracker.index_call_sites end diff --git a/lib/brakeman/tracker.rb b/lib/brakeman/tracker.rb index da0097730b7fdd558371a95797c12f9b409a7b51..68dc7074c17f4c4522c4da2fdda5c2b5f4e345dc 100644 --- a/lib/brakeman/tracker.rb +++ b/lib/brakeman/tracker.rb @@ -20,9 +20,11 @@ class Brakeman::Tracker # #The Processor argument is only used by other Processors #that might need to access it. - def initialize processor = nil, options = {} + def initialize(app_tree, processor = nil, options = {}) + @app_tree = app_tree @processor = processor @options = options + @config = {} @templates = {} @controllers = {} @@ -67,7 +69,7 @@ class Brakeman::Tracker #Run a set of checks on the current information. Results will be stored #in Tracker#checks. def run_checks - @checks = Brakeman::Checks.run_checks(self) + @checks = Brakeman::Checks.run_checks(@app_tree, self) @end_time = Time.now @duration = @end_time - @start_time @@ -147,7 +149,7 @@ class Brakeman::Tracker #Returns a Report with this Tracker's information def report - Brakeman::Report.new(self) + Brakeman::Report.new(@app_tree, self) end def index_call_sites diff --git a/lib/brakeman/util.rb b/lib/brakeman/util.rb index 6035ea1f8867dfb895e1167df2d247a43b6c4369..2a4e9c9a30c633ac3be69ddeb842e131019e6daa 100644 --- a/lib/brakeman/util.rb +++ b/lib/brakeman/util.rb @@ -75,7 +75,7 @@ module Brakeman::Util end index += 2 end - + hash << key << value hash @@ -107,8 +107,8 @@ module Brakeman::Util #Check if _exp_ represents a hash: s(:hash, {...}) #This also includes pseudo hashes params, session, and cookies. def hash? exp - exp.is_a? Sexp and (exp.node_type == :hash or - exp.node_type == :params or + exp.is_a? Sexp and (exp.node_type == :hash or + exp.node_type == :params or exp.node_type == :session or exp.node_type == :cookies) end @@ -331,10 +331,10 @@ module Brakeman::Util #Return array of lines surrounding the warning location from the original #file. - def context_for warning, tracker = nil + def context_for app_tree, warning, tracker = nil file = file_for warning, tracker context = [] - return context unless warning.line and file and File.exist? file + return context unless warning.line and file and @app_tree.path_exists? file current_line = 0 start_line = warning.line - 5 @@ -385,5 +385,5 @@ module Brakeman::Util output << CSV.generate_line(row.cells.map{|cell| cell.to_s.strip}) end output - end + end end diff --git a/test/tests/test_brakeman.rb b/test/tests/test_brakeman.rb index a6f737c508a74b058533799d8264985910b49bcc..01c71c0d01783709ecd2f3bc9a36510103a32bf9 100644 --- a/test/tests/test_brakeman.rb +++ b/test/tests/test_brakeman.rb @@ -18,10 +18,12 @@ end class BaseCheckTests < Test::Unit::TestCase FakeTracker = Struct.new(:config) + FakeAppTree = Struct.new(:root) def setup @tracker = FakeTracker.new - @check = Brakeman::BaseCheck.new @tracker + app_tree = FakeAppTree.new + @check = Brakeman::BaseCheck.new app_tree, @tracker end def version_between? version, high, low