diff --git a/lib/brakeman/checks/check_basic_auth.rb b/lib/brakeman/checks/check_basic_auth.rb index 2e674c4cf68a35a22f0cdb6388f184a006718f99..3c3f7574519bd13b3e222c6ef24659f1649dac27 100644 --- a/lib/brakeman/checks/check_basic_auth.rb +++ b/lib/brakeman/checks/check_basic_auth.rb @@ -12,6 +12,11 @@ class Brakeman::CheckBasicAuth < Brakeman::BaseCheck def run_check return if version_between? "0.0.0", "3.0.99" + check_basic_auth_filter + check_basic_auth_request + end + + def check_basic_auth_filter controllers = tracker.controllers.select do |name, c| c[:options][:http_basic_authenticate_with] end @@ -21,10 +26,10 @@ class Brakeman::CheckBasicAuth < Brakeman::BaseCheck if pass = get_password(call) and string? pass warn :controller => name, - :warning_type => "Basic Auth", + :warning_type => "Basic Auth", :warning_code => :basic_auth_password, :message => "Basic authentication password stored in source code", - :code => call, + :code => call, :confidence => 0, :file => controller[:file] @@ -34,6 +39,46 @@ class Brakeman::CheckBasicAuth < Brakeman::BaseCheck end end + # Look for + # authenticate_or_request_with_http_basic do |username, password| + # username == "foo" && password == "bar" + # end + def check_basic_auth_request + tracker.find_call(:target => nil, :method => :authenticate_or_request_with_http_basic).each do |result| + if include_password_literal? result + warn :result => result, + :code => @include_password, + :warning_type => "Basic Auth", + :warning_code => :basic_auth_password, + :message => "Basic authentication password stored in source code", + :confidence => 0 + end + end + end + + # Check if the block of a result contains a comparison of password to string + def include_password_literal? result + @password_var = result[:block_args].last + @include_password = false + process result[:block] + @include_password + end + + # Looks for :== calls on password var + def process_call exp + target = exp.target + + if node_type?(target, :lvar) and + target.value == @password_var and + exp.method == :== and + string? exp.first_arg + + @include_password = exp + end + + exp + end + def get_password call arg = call.first_arg diff --git a/test/apps/rails3.1/app/controllers/admin_controller.rb b/test/apps/rails3.1/app/controllers/admin_controller.rb index 80b8494fbcce152c2c121e41e1e6428c4e3debf1..fbbb252c0201e6edcb631a81b265ceec9dd4f7e4 100644 --- a/test/apps/rails3.1/app/controllers/admin_controller.rb +++ b/test/apps/rails3.1/app/controllers/admin_controller.rb @@ -17,4 +17,12 @@ class AdminController < ApplicationController some_method(params[:class]).constantize end + + def authenticate_user! + correct_password = "7001337" + + authenticate_or_request_with_http_basic do |username, password| + username == "foo" && password == correct_password + end + end end diff --git a/test/tests/rails31.rb b/test/tests/rails31.rb index f176c795b327b42b49588f7840d9125385210fb9..ccedc973527253cdffbd81d48b5d0ea097c80fad 100644 --- a/test/tests/rails31.rb +++ b/test/tests/rails31.rb @@ -15,7 +15,7 @@ class Rails31Tests < Test::Unit::TestCase :model => 3, :template => 22, :controller => 4, - :warning => 71 } + :warning => 72 } end def test_without_protection @@ -117,6 +117,17 @@ class Rails31Tests < Test::Unit::TestCase :file => /users_controller\.rb/ end + def test_basic_auth_in_method_with_password + assert_warning :type => :warning, + :warning_code => 9, + :fingerprint => "f2698a4ca148f43a8f77901a57371b6253f450d50ad388de588f32b7dbeb8937", + :warning_type => "Basic Auth", + :line => 25, + :message => /^Basic\ authentication\ password\ stored\ in\ /, + :confidence => 0, + :relative_path => "app/controllers/admin_controller.rb" + end + def test_translate_bug assert_warning :type => :warning, :warning_type => "Cross Site Scripting",