Report on: allow_forgery_protection = false

[forced-request]

Is your feature request related to a problem? Please describe.

@presidentbeef and I have talked about this a bit already, but the gist is that I recommend adding a rule to check for any changes to allow_forgery_protection via global configuration.

In Rails 6 users can explicitly opt-out of CSRF protections at the config level by setting config.action_controller.allow_forgery_protection = false, as seen in the example below.

When this situation occurs Brakeman fails to warn about it, resulting in the application being completely vulnerable to CSRF.

require_relative 'boot'

require 'rails/all'

# Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production.
Bundler.require(*Rails.groups)

module Test
  class Application < Rails::Application
    # Initialize configuration defaults for originally generated Rails version.
    config.load_defaults 6.0

    config.action_controller.allow_forgery_protection = false

    config.active_record.schema_format = :sql
  end
end

Describe the solution you'd like

• Brakeman should warn if allow_forgery_protection is disabled.
• Brakeman should warn (with lower severity) if per_form_csrf_tokens is disabled

Describe alternatives you've considered N/A

Additional context Add any other context or screenshots about the feature request here.

[jamgregory]

I think that sounds like a sensible idea to me. I was also wondering if you could bundle the following check in as well?

skip_before_action :verify_authenticity_token

I think it falls into the same category - it's explicitly stopping the check of the CSRF token, and should probably be warned about by Brakeman.

[jamgregory]

Reading more into this, I wonder if it's also worth flagging up cases where protect_from_forgery could be turned off, e.g.:

protect_from_forgery except: :index
protect_from_forgery only: :show
protect_from_forgery if: -> { ... }
protect_from_forgery unless: -> { ... }