ruby.rails.security.brakeman.check-before-filter.check-before-filter

profile photo of semgrepsemgrep
Author
unknown
Download Count*
LGPL Commons Clause
License

Disabled-by-default Rails controller checks make it much easier to introduce access control mistakes. Prefer an allowlist approach with :only => [...] rather than except: => [...]

Run Locally

Run in CI

Semgrep CI docs

Defintion

Edit RuleView Source
Open in Playground 
rules:
  - id: check-before-filter
    mode: search
    patterns:
      - pattern-either:
          - pattern: |
              skip_filter ..., :except => $ARGS
          - pattern: |
              skip_before_filter ..., :except => $ARGS
    message: "Disabled-by-default Rails controller checks make it much easier to
      introduce access control mistakes. Prefer an allowlist approach with
      `:only => [...]` rather than `except: => [...]`"
    languages:
      - ruby
    severity: ERROR
    metadata:
      source-rule-url: https://github.com/presidentbeef/brakeman/blob/main/lib/brakeman/checks/check_skip_before_filter.rb
      category: security
      cwe:
        - "CWE-284: Improper Access Control"
      owasp:
        - A05:2017 - Broken Access Control
        - A01:2021 - Broken Access Control
      technology:
        - ruby
        - rails
      references:
        - https://owasp.org/Top10/A01_2021-Broken_Access_Control
      subcategory:
        - vuln
      impact: MEDIUM
      likelihood: MEDIUM
      confidence: MEDIUM
      license: Commons Clause License Condition v1.0[LGPL-2.1-only]
      vulnerability_class:
        - Improper Authorization

Examples

check-before-filter.rb

class BadController < ApplicationController
  #Examples of skipping important filters with a blacklist instead of whitelist
  # ruleid: check-before-filter
  skip_before_filter :login_required, :except => :do_admin_stuff
  # ruleid: check-before-filter
  skip_filter :authenticate_user!, :except => :do_admin_stuff
  # ruleid: check-before-filter
  skip_before_filter :require_user, :except => [:do_admin_stuff, :do_other_stuff]

    def do_admin_stuff
        #do some stuff
    end
    
    def do_anonymous_stuff
      # do some stuff
    end
end

class GoodController < ApplicationController
  #Examples of skipping important filters with a blacklist instead of whitelist
  # ok: check-before-filter
  skip_before_filter :login_required, :only => :do_anonymous_stuff
  # ok: check-before-filter
  skip_filter :authenticate_user!, :only => :do_anonymous_stuff
  # ok: check-before-filter
  skip_before_filter :require_user, :only => [:do_anonymous_stuff, :do_nocontext_stuff]

    def do_admin_stuff
        #do some stuff
    end
    
    def do_anonymous_stuff
      # do some stuff
    end

    def do_nocontext_stuff
      # do some stuff
    end
end