Class: Scryer::Rules::IdorRule

Inherits:
Scryer::Rule show all
Defined in:
lib/scryer/rules/idor_rule.rb

Overview

Flags Model.find(params[...]) / Model.find_by(...params...) inside a controller where the receiver is a bare, unnamespaced constant (not scoped through e.g. current_user.things.find(...)) and no authorization call (authorize, authorize!, policy_scope, can?, cannot? — the common Pundit/CanCanCan method names) appears anywhere else in the same controller class.

This is the least precise rule in the gem, by nature of the problem: whether a given find is actually scoped to the current user is a question about the whole app's authorization model, not something visible from one file's AST. Expect real false positives — e.g. an admin-only controller already gated by a class-level before_action, or a genuinely global/unowned model (Country.find(params[:id])). Treat every finding as "worth a second look," not a confirmed bug — same spirit as CsrfProtectionRule's class-wide safeguard check, applied to a harder problem.

Constant Summary collapse

FINDER_METHODS = 
%w[find find_by find_by!].freeze
NON_MODEL_RECEIVERS =

Same reasoning as MassAssignmentRule::NON_MODEL_RECEIVERS: common stdlib/gem constants with their own .find-style methods that have nothing to do with an ActiveRecord model lookup.

%w[
  Struct OpenStruct Data Class Module BCrypt OpenSSL Net URI Digest
  JSON YAML Marshal String Array Hash Integer Float Symbol Comparable
  Enumerable File Dir
].freeze
AUTHORIZATION_METHODS = 
%w[authorize authorize! policy_scope can? cannot?].freeze

Instance Attribute Summary

Attributes inherited from Scryer::Rule

#file, #sexp, #source

Instance Method Summary collapse

Methods inherited from Scryer::Rule

inherited, #initialize

Constructor Details

This class inherits a constructor from Scryer::Rule

Instance Method Details

#scan ⇒ Object 



38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
 
# File 'lib/scryer/rules/idor_rule.rb', line 38

def scan
  findings = []

  Ast.each_node(sexp) do |node|
    next unless Ast.tagged?(node, :class)

    class_name = Ast.ident_text(node[1].is_a?(Array) ? node[1][1] : nil)
    next unless class_name.to_s.end_with?("Controller")

    body = node[3]
    next if each_call_names(body).any? { |name| AUTHORIZATION_METHODS.include?(name) }

    each_unscoped_find(body).each do |find_node, method_name|
      line = Ast.line_of(find_node)
      findings << finding(
        line: line,
        message: "`#{method_name}` looks up a record directly from `params`, and " \
                  "`#{class_name}` has no visible authorization check (no `authorize`, " \
                  "`policy_scope`, or `can?`/`cannot?` anywhere in it) — if this record " \
                  "belongs to a specific user/account, another user may be able to view or " \
                  "modify it just by changing the id in the request.",
        suggested_fix: "Scope the lookup to the current actor instead of the bare model, " \
                        "e.g. `current_user.things.#{method_name}(params[:id])`, or add an " \
                        "explicit authorization check (`authorize @thing` for Pundit, " \
                        "`authorize! :show, @thing` for CanCanCan) before using the record."
      )
    end
  end

  findings
end