Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional CSRF deactivation checks #1720

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/brakeman/checks/base_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,14 @@ def version_between? low_version, high_version, current_version = nil
tracker.config.version_between? low_version, high_version, current_version
end

def version_gte? low_version, current_version = nil
tracker.config.version_gte? low_version, current_version
end

def version_lte? high_version, current_version = nil
tracker.config.version_lte? high_version, current_version
end

def gemfile_or_environment gem_name = :rails
if gem_name and info = tracker.config.get_gem(gem_name.to_sym)
info
Expand Down
10 changes: 9 additions & 1 deletion lib/brakeman/checks/check_forgery_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ class Brakeman::CheckForgerySetting < Brakeman::BaseCheck
@description = "Verifies that protect_from_forgery is enabled in direct subclasses of ActionController::Base"

def run_check
unless tracker.config.allow_forgery_protection?
csrf_warning :controller => nil,
:warning_code => :csrf_protection_disabled,
:file => "config/environments/production.rb",
:confidence => :high,
:message => msg("CSRF protection has been deactivated by ", msg_code("allow_forgery_protection"))
end

return if tracker.config.default_protect_from_forgery?

tracker.controllers
Expand All @@ -21,7 +29,7 @@ def run_check
:message => msg(msg_code("protect_from_forgery"), " should be called in ", msg_code(name)),
:file => controller.file,
:line => controller.top_line
elsif version_between? "4.0.0", "100.0.0" and forgery_opts = controller.options[:protect_from_forgery]
elsif version_gte? "4.0.0" and forgery_opts = controller.options[:protect_from_forgery]
unless forgery_opts.is_a?(Array) and sexp?(forgery_opts.first) and
access_arg = hash_access(forgery_opts.first.first_arg, :with) and symbol? access_arg and
access_arg.value == :exception
Expand Down
34 changes: 27 additions & 7 deletions lib/brakeman/tracker/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,18 @@ def initialize tracker
end

def default_protect_from_forgery?
if version_between? "5.2.0.beta1", "9.9.9"
if @rails.dig(:action_controller, :default_protect_from_forgery) == Sexp.new(:false)
return false
else
return true
end
# This defaults to true in Rails >= 5.2
if version_gte?("5.2.0.beta1")
return !(@rails.dig(:action_controller, :default_protect_from_forgery) == Sexp.new(:false))
end

false
end

def allow_forgery_protection?
!(@rails.dig(:action_controller, :allow_forgery_protection) == Sexp.new(:false))
end

def erubis?
@erubis
end
Expand Down Expand Up @@ -153,11 +154,28 @@ def version_between? low_version, high_version, current_version = nil
current.between?(low, high)
end

# Returns true if RAILS_VERSION >= low_version
# Return false if the Rails version is unknown
def version_gte? low_version, current_version = nil
current_version ||= rails_version
return false unless current_version

Gem::Version.new(current_version) >= Gem::Version.new(low_version)
end

# Returns true if RAILS_VERSION <= high_version
# Return false if the Rails version is unknown
def version_lte? high_version, current_version = nil
current_version ||= rails_version
return false unless current_version

Gem::Version.new(current_version) <= Gem::Version.new(high_version)
end

def session_settings
@rails.dig(:action_controller, :session)
end


# Set Rails config option value
# where path is an array of attributes, e.g.
#
Expand Down Expand Up @@ -196,6 +214,8 @@ def load_rails_defaults
true_value = Sexp.new(:true)
false_value = Sexp.new(:false)

set_rails_config(value: true_value, path: [:action_controller, :allow_forgery_protection])

if version >= 5.0
set_rails_config(value: true_value, path: [:action_controller, :per_form_csrf_tokens])
set_rails_config(value: true_value, path: [:action_controller, :forgery_protection_origin_check])
Expand Down
50 changes: 50 additions & 0 deletions test/tests/brakeman.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ def version_between? version, low, high
@check.send(:version_between?, low, high)
end

def version_gte? version, low
@tracker.config.set_rails_version version
@check.send(:version_gte?, low)
end

def version_lte? version, high
@tracker.config.set_rails_version version
@check.send(:version_lte?, high)
end

def lts_version? version, low
if version
@tracker.config.add_gem :"railslts-version", version, nil, nil
Expand Down Expand Up @@ -146,6 +156,46 @@ def test_version_between_pre_release
assert version_between?("3.2.9.rc2", "3.2.5", "4.0.0")
end

def test_version_gte
assert version_gte?("2.3.8", "2.3.0")
assert version_gte?("2.3.8", "2.3.8")
assert version_gte?("2.3.8", "1.0.0")
end

def test_version_not_gte
assert_equal false, version_gte?("3.2.1", "3.2.2")
assert_equal false, version_gte?("3.2.1", "4.0.0")
assert_equal false, version_gte?("3.2.1", "9.9.9")
end

def test_version_gte_longer
assert_equal false, version_gte?("1.0.1", "1.0.1.1")
end

def test_version_gte_pre_release
assert version_gte?("3.2.9.rc2", "3.2.8")
end

def test_version_lte
assert version_lte?("2.3.8", "2.3.9")
assert version_lte?("2.3.8", "2.4.0")
assert version_lte?("2.3.8", "3.0.0")
end

def test_version_not_lte
assert_equal false, version_lte?("3.2.1", "3.2.0")
assert_equal false, version_lte?("3.2.1", "3.0.0")
assert_equal false, version_lte?("3.2.1", "0.0.0")
end

def test_version_lte_longer
assert_equal false, version_lte?("1.0.1.1", "1.0.1")
end

def test_version_lte_pre_release
assert version_lte?("3.2.9.rc2", "3.2.9")
end

def test_lts_version
@tracker.config.set_rails_version "2.3.18"
assert lts_version? '2.3.18.6', '2.3.18.6'
Expand Down