ruby.rails.security.brakeman.check-redirect-to.check-redirect-to

profile photo of semgrepsemgrep
Author
unknown
Download Count*

Found potentially unsafe handling of redirect behavior $X. Do not pass params to redirect_to without the :only_path => true hash value.

Run Locally

Run in CI

Defintion

rules:
  - id: check-redirect-to
    mode: taint
    pattern-sources:
      - patterns:
          - pattern-either:
              - pattern: params
              - pattern: cookies
              - pattern: request.env
              - pattern: url_for(params[...],...,:only_path => false,...)
    pattern-sanitizers:
      - patterns:
          - pattern-either:
              - patterns:
                  - pattern: |
                      $F(...)
                  - metavariable-pattern:
                      metavariable: $F
                      patterns:
                        - pattern-not-regex: (params|url_for|cookies|request.env|permit|redirect_to)
              - pattern: |
                  params.merge! :only_path => true
                  ...
              - pattern: |
                  params.slice(...)
                  ...
              - pattern: |
                  redirect_to [...]
              - patterns:
                  - pattern: |
                      $MODEL. ... .$M(...)
                      ...
                  - metavariable-regex:
                      metavariable: $MODEL
                      regex: "[A-Z]\\w+"
                  - metavariable-regex:
                      metavariable: $M
                      regex: (all|create|find|find_by|find_by_sql|first|last|new|from|group|having|joins|lock|order|reorder|select|where|take)
              - patterns:
                  - pattern: |
                      params.$UNSAFE_HASH.merge(...,:only_path => true,...)
                      ...
                  - metavariable-regex:
                      metavariable: $UNSAFE_HASH
                      regex: to_unsafe_h(ash)?
              - patterns:
                  - pattern: params.permit(...,$X,...)
                  - metavariable-pattern:
                      metavariable: $X
                      patterns:
                        - pattern-not-regex: (host|port|(sub)?domain)
    pattern-sinks:
      - patterns:
          - pattern: $X
          - pattern-inside: |
              redirect_to $X, ...
          - pattern-not-regex: params\.\w+(?<!permit)\(.*?\)
    message: Found potentially unsafe handling of redirect behavior $X. Do not pass
      `params` to `redirect_to` without the `:only_path => true` hash value.
    languages:
      - ruby
    severity: WARNING
    metadata:
      source-rule-url: https://github.com/presidentbeef/brakeman/blob/main/lib/brakeman/checks/check_redirect.rb
      category: security
      cwe:
        - "CWE-601: URL Redirection to Untrusted Site ('Open Redirect')"
      technology:
        - ruby
        - rails
      references:
        - https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html
      owasp:
        - A01:2021 - Broken Access Control
      subcategory:
        - vuln
      likelihood: MEDIUM
      impact: MEDIUM
      confidence: MEDIUM
      license: Commons Clause License Condition v1.0[LGPL-2.1-only]
      vulnerability_class:
        - Open Redirect

Examples

check-redirect-to.rb

class BaseController < ActionController::Base
  def test_redirect
    params[:action] = :index
    #ruleid: check-redirect-to
    redirect_to params
  end

  def redirect_to_strong_params
    # ruleid: check-redirect-to
    redirect_to params.permit(:domain) # should warn
    # ok: check-redirect-to
    redirect_to params.permit(:page, :sort) # should not warn
    # ok: check-redirect-to
    redirect_to [params.permit(:domain)]
  end

  def test_only_path_wrong
    # ruleid: check-redirect-to
    redirect_to params[:user], :only_path => true #This should still warn
  end
  def test_only_path_correct
    params.merge! :only_path => true
    # ok: check-redirect-to
    redirect_to params
  end

  def wrong_redirect_only_path
    # ruleid: check-redirect-to
    redirect_to(params.bla.merge(:only_path => true, :display => nil))
  end

  def redirect_only_path_with_unsafe_hash
    # ok: check-redirect-to
    redirect_to(params.to_unsafe_hash.merge(:only_path => true, :display => nil))
  end

  def redirect_only_path_with_unsafe_h
    # ok: check-redirect-to
    redirect_to(params.to_unsafe_h.merge(:only_path => true, :display => nil))
  end
end