From f15024d73e70d18d934d15018b320f2b853a94f7 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Wed, 30 Nov 2011 16:07:04 -0800 Subject: [PATCH] Add more debug messages to checks --- lib/brakeman/checks/base_check.rb | 5 +++++ lib/brakeman/checks/check_cross_site_scripting.rb | 5 ++++- lib/brakeman/checks/check_default_routes.rb | 2 ++ lib/brakeman/checks/check_evaluation.rb | 2 ++ lib/brakeman/checks/check_execute.rb | 3 +++ lib/brakeman/checks/check_file_access.rb | 4 ++++ lib/brakeman/checks/check_mail_to.rb | 2 ++ lib/brakeman/checks/check_mass_assignment.rb | 2 ++ lib/brakeman/checks/check_quote_table_name.rb | 2 ++ lib/brakeman/checks/check_redirect.rb | 3 +++ lib/brakeman/checks/check_render.rb | 2 ++ lib/brakeman/checks/check_send_file.rb | 2 ++ lib/brakeman/checks/check_sql.rb | 5 +++++ lib/brakeman/checks/check_strip_tags.rb | 2 ++ lib/brakeman/checks/check_translate_bug.rb | 2 ++ lib/brakeman/checks/check_without_protection.rb | 2 ++ 16 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index b04bd4d6..c2faed69 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -19,6 +19,7 @@ class Brakeman::BaseCheck < SexpProcessor @tracker = tracker @string_interp = false @current_set = nil + @debug_mode = tracker.options[:debug] @current_template = @current_module = @current_class = @current_method = nil self.strict = false self.auto_shift_type = false @@ -376,4 +377,8 @@ class Brakeman::BaseCheck < SexpProcessor "config/environment.rb" end end + + def debug_info msg + Kernel.warn msg if @debug_mode + end end diff --git a/lib/brakeman/checks/check_cross_site_scripting.rb b/lib/brakeman/checks/check_cross_site_scripting.rb index 3dca6c3b..2008b49e 100644 --- a/lib/brakeman/checks/check_cross_site_scripting.rb +++ b/lib/brakeman/checks/check_cross_site_scripting.rb @@ -62,9 +62,12 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck tracker.each_template do |name, template| @current_template = template - template[:outputs].each do |out| + debug_info "Checking #{name} for direct XSS" + unless check_for_immediate_xss out + debug_info "Checking #{name} for indirect XSS" + @matched = false @mark = false process out diff --git a/lib/brakeman/checks/check_default_routes.rb b/lib/brakeman/checks/check_default_routes.rb index f408eaca..7d9ddf9c 100644 --- a/lib/brakeman/checks/check_default_routes.rb +++ b/lib/brakeman/checks/check_default_routes.rb @@ -15,6 +15,8 @@ class Brakeman::CheckDefaultRoutes < Brakeman::BaseCheck :confidence => CONFIDENCE[:high], :file => "#{tracker.options[:app_path]}/config/routes.rb" else #Report each controller separately + debug_info "Checking each controller for default routes" + tracker.routes.each do |name, actions| if actions.is_a? Array and actions[0] == :allow_all_actions warn :controller => name, diff --git a/lib/brakeman/checks/check_evaluation.rb b/lib/brakeman/checks/check_evaluation.rb index 4f41f182..432130a5 100644 --- a/lib/brakeman/checks/check_evaluation.rb +++ b/lib/brakeman/checks/check_evaluation.rb @@ -7,8 +7,10 @@ class Brakeman::CheckEvaluation < Brakeman::BaseCheck #Process calls def run_check + debug_info "Finding eval-like calls" calls = tracker.find_call nil, [:eval, :instance_eval, :class_eval, :module_eval] + debug_info "Processing eval-like calls" calls.each do |call| process_result call end diff --git a/lib/brakeman/checks/check_execute.rb b/lib/brakeman/checks/check_execute.rb index c0dd5302..0afacb85 100644 --- a/lib/brakeman/checks/check_execute.rb +++ b/lib/brakeman/checks/check_execute.rb @@ -14,10 +14,13 @@ class Brakeman::CheckExecute < Brakeman::BaseCheck #Check models, controllers, and views for command injection. def run_check + debug_info "Finding system calls using ``" check_for_backticks tracker + debug_info "Finding other system calls" calls = tracker.find_call [:IO, :Open3, :Kernel, []], [:exec, :popen, :popen3, :syscall, :system] + debug_info "Processing system calls" calls.each do |result| process result end diff --git a/lib/brakeman/checks/check_file_access.rb b/lib/brakeman/checks/check_file_access.rb index f18612ba..0a890aab 100644 --- a/lib/brakeman/checks/check_file_access.rb +++ b/lib/brakeman/checks/check_file_access.rb @@ -6,12 +6,16 @@ class Brakeman::CheckFileAccess < Brakeman::BaseCheck Brakeman::Checks.add self def run_check + debug_info "Finding possible file access" methods = tracker.find_call [:Dir, :File, :IO, :Kernel, :"Net::FTP", :"Net::HTTP", :PStore, :Pathname, :Shell, :YAML], [:[], :chdir, :chroot, :delete, :entries, :foreach, :glob, :install, :lchmod, :lchown, :link, :load, :load_file, :makedirs, :move, :new, :open, :read, :read_lines, :rename, :rmdir, :safe_unlink, :symlink, :syscopy, :sysopen, :truncate, :unlink] + debug_info "Finding calls to load()" methods.concat tracker.find_call [], [:load] + debug_info "Finding calls using FileUtils" methods.concat tracker.find_call(:FileUtils, nil) + debug_info "Processing found calls" methods.each do |call| process_result call end diff --git a/lib/brakeman/checks/check_mail_to.rb b/lib/brakeman/checks/check_mail_to.rb index 8890c52d..2b163ba6 100644 --- a/lib/brakeman/checks/check_mail_to.rb +++ b/lib/brakeman/checks/check_mail_to.rb @@ -29,6 +29,8 @@ class Brakeman::CheckMailTo < Brakeman::BaseCheck #Check for javascript encoding of mail_to address # mail_to email, name, :encode => :javascript def mail_to_javascript? + debug_info "Checking calls to mail_to for javascript encoding" + tracker.find_call([], :mail_to).each do |result| call = result[-1] args = call[-1] diff --git a/lib/brakeman/checks/check_mass_assignment.rb b/lib/brakeman/checks/check_mass_assignment.rb index 22854b02..1828c550 100644 --- a/lib/brakeman/checks/check_mass_assignment.rb +++ b/lib/brakeman/checks/check_mass_assignment.rb @@ -20,6 +20,7 @@ class Brakeman::CheckMassAssignment < Brakeman::BaseCheck @results = Set.new + debug_info "Finding possible mass assignment calls on #{models.length} models" calls = tracker.find_call models, [:new, :attributes=, :update_attribute, @@ -28,6 +29,7 @@ class Brakeman::CheckMassAssignment < Brakeman::BaseCheck :create, :create!] + debug_info "Processing possible mass assignment calls" calls.each do |result| process result end diff --git a/lib/brakeman/checks/check_quote_table_name.rb b/lib/brakeman/checks/check_quote_table_name.rb index f4ad8ae3..dfc3814e 100644 --- a/lib/brakeman/checks/check_quote_table_name.rb +++ b/lib/brakeman/checks/check_quote_table_name.rb @@ -30,6 +30,8 @@ class Brakeman::CheckQuoteTableName < Brakeman::BaseCheck end def uses_quote_table_name? + debug_info "Finding calls to quote_table_name()" + not tracker.find_call([], :quote_table_name).empty? end end diff --git a/lib/brakeman/checks/check_redirect.rb b/lib/brakeman/checks/check_redirect.rb index b45bc500..7b627778 100644 --- a/lib/brakeman/checks/check_redirect.rb +++ b/lib/brakeman/checks/check_redirect.rb @@ -10,6 +10,8 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck Brakeman::Checks.add self def run_check + debug_info "Finding calls to redirect_to()" + @tracker.find_call(nil, :redirect_to).each do |c| process c end @@ -43,6 +45,7 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck #which can be used to enable/disable reporting output of method calls which use #user input as arguments. def include_user_input? call + debug_info "Checking if call includes user input" if tracker.options[:ignore_redirect_to_model] and call? call[3][1] and call[3][1][2] == :new and call[3][1][1] diff --git a/lib/brakeman/checks/check_render.rb b/lib/brakeman/checks/check_render.rb index b1b786c8..c2ce6886 100644 --- a/lib/brakeman/checks/check_render.rb +++ b/lib/brakeman/checks/check_render.rb @@ -5,12 +5,14 @@ class Brakeman::CheckRender < Brakeman::BaseCheck Brakeman::Checks.add self def run_check + debug_info "Checking all method bodies for calls to render()" tracker.each_method do |src, class_name, method_name| @current_class = class_name @current_method = method_name process src end + debug_info "Checking all templates for calls to render()" tracker.each_template do |name, template| @current_template = template process template[:src] diff --git a/lib/brakeman/checks/check_send_file.rb b/lib/brakeman/checks/check_send_file.rb index 1d8bc026..5e4fc144 100644 --- a/lib/brakeman/checks/check_send_file.rb +++ b/lib/brakeman/checks/check_send_file.rb @@ -6,6 +6,8 @@ class Brakeman::CheckSendFile < Brakeman::CheckFileAccess Brakeman::Checks.add self def run_check + debug_info "Finding all calls to send_file()" + methods = tracker.find_call nil, :send_file methods.each do |call| diff --git a/lib/brakeman/checks/check_sql.rb b/lib/brakeman/checks/check_sql.rb index c6740d78..989f07b5 100644 --- a/lib/brakeman/checks/check_sql.rb +++ b/lib/brakeman/checks/check_sql.rb @@ -14,12 +14,17 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck def run_check @rails_version = tracker.config[:rails_version] + + debug_info "Finding possible SQL calls on models" calls = tracker.find_model_find tracker.models.keys + debug_info "Finding possible SQL calls with no target" calls.concat tracker.find_call([], /^(find.*|last|first|all|count|sum|average|minumum|maximum|count_by_sql)$/) + debug_info "Finding possible SQL calls using constantized()" calls.concat tracker.find_model_find(nil).select { |result| constantize_call? result } + debug_info "Processing possible SQL calls" calls.each do |c| process c end diff --git a/lib/brakeman/checks/check_strip_tags.rb b/lib/brakeman/checks/check_strip_tags.rb index d68b2376..5e0c95e9 100644 --- a/lib/brakeman/checks/check_strip_tags.rb +++ b/lib/brakeman/checks/check_strip_tags.rb @@ -24,6 +24,8 @@ class Brakeman::CheckStripTags < Brakeman::BaseCheck end def uses_strip_tags? + debug_info "Finding calls to strip_tags()" + not tracker.find_call([], :strip_tags).empty? end end diff --git a/lib/brakeman/checks/check_translate_bug.rb b/lib/brakeman/checks/check_translate_bug.rb index 887f99eb..f3415981 100644 --- a/lib/brakeman/checks/check_translate_bug.rb +++ b/lib/brakeman/checks/check_translate_bug.rb @@ -35,6 +35,8 @@ class Brakeman::CheckTranslateBug < Brakeman::BaseCheck end def uses_translate? + debug_info "Finding calls to translate() or t()" + not tracker.find_call([], [:t, :translate]).empty? end end diff --git a/lib/brakeman/checks/check_without_protection.rb b/lib/brakeman/checks/check_without_protection.rb index 7680378b..3335c264 100644 --- a/lib/brakeman/checks/check_without_protection.rb +++ b/lib/brakeman/checks/check_without_protection.rb @@ -23,6 +23,7 @@ class Brakeman::CheckWithoutProtection < Brakeman::BaseCheck @results = Set.new + debug_info "Finding all mass assignments" calls = tracker.find_call models, [:new, :attributes=, :update_attribute, @@ -31,6 +32,7 @@ class Brakeman::CheckWithoutProtection < Brakeman::BaseCheck :create, :create!] + debug_info "Processing all mass assignments" calls.each do |result| process result end -- GitLab