ruby.rails.security.brakeman.check-http-verb-confusion.check-http-verb-confusion

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

Found an improperly constructed control flow block with request.get?. Rails will route HEAD requests as GET requests but they will fail the request.get? check, potentially causing unexpected behavior unless an elif condition is used.

Run Locally

Run in CI

Semgrep CI docs

Defintion

Edit RuleView Source
Open in Playground 
rules:
  - id: check-http-verb-confusion
    mode: search
    patterns:
      - pattern: |
          if request.get?
            ...
          else
            ...
          end
      - pattern-not-inside: |
          if ...
          elsif ...
            ...
          end
    message: Found an improperly constructed control flow block with `request.get?`.
      Rails will route HEAD requests as GET requests but they will fail the
      `request.get?` check, potentially causing unexpected behavior unless an
      `elif` condition is used.
    languages:
      - ruby
    severity: ERROR
    metadata:
      source-rule-url: https://github.com/presidentbeef/brakeman/blob/main/lib/brakeman/checks/check_verb_confusion.rb
      category: security
      cwe:
        - "CWE-650: Trusting HTTP Permission Methods on the Server Side"
      owasp:
        - A04:2021 - Insecure Design
      technology:
        - ruby
        - rails
      references:
        - https://github.com/presidentbeef/brakeman/blob/main/test/apps/rails6/app/controllers/accounts_controller.rb
      subcategory:
        - vuln
      likelihood: HIGH
      impact: MEDIUM
      confidence: MEDIUM
      license: Commons Clause License Condition v1.0[LGPL-2.1-only]
      vulnerability_class:
        - Other

Examples

check-http-verb-confusion.rb

class AccountsController < ApplicationController
  def login
    # ruleid: check-http-verb-confusion
    if request.get?
      # Do something benign
    else
      # Do something sensitive because it's a POST
      # but actually it could be a HEAD :(
    end
  end

  def auth_something 
    # Does not warn because there is an elsif clause
    # ok: check-http-verb-confusion
    if request.get?
      # Do something benign
    elsif request.post?
      # Do something sensitive because it's a POST
    end

    if request.post?
      # Do something sensitive because it's a POST
    elsif request.get?
      # Do something benign
    end
  end
end