Skip to content

Commit

Permalink
Merge pull request #1799 from presidentbeef/ransack
Browse files Browse the repository at this point in the history
Add check for unrestricted search using Ransack
  • Loading branch information
presidentbeef authored Oct 6, 2023
2 parents f48d572 + 51ec9c2 commit 9f6eb56
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 2 deletions.
51 changes: 51 additions & 0 deletions lib/brakeman/checks/check_ransack.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require 'brakeman/checks/base_check'

class Brakeman::CheckRansack < Brakeman::BaseCheck
Brakeman::Checks.add self

@description = "Checks for dangerous use of the Ransack library"

def run_check
return unless version_between? "0.0.0", "3.99", tracker.config.gem_version(:ransack)
check_ransack_calls
end

def check_ransack_calls
tracker.find_call(method: :ransack, nested: true).each do |result|
call = result[:call]
arg = call.first_arg

# If an allow list is defined anywhere in the
# class or super classes, consider it safe
class_name = result[:chain].first

next if ransackable_allow_list?(class_name)

if input = has_immediate_user_input?(arg)
confidence = if tracker.find_class(class_name).nil?
confidence = :low
elsif result[:location][:file].relative.include? 'admin'
confidence = :medium
else
confidence = :high
end

message = msg('Unrestricted search using ', msg_code('ransack'), ' library called with ', msg_input(input), '. Limit search by defining ', msg_code('ransackable_attributes'), ' and ', msg_code('ransackable_associations'), ' methods in class or upgrade Ransack to version 4.0.0 or newer')

warn result: result,
warning_type: 'Missing Authorization',
warning_code: :ransack_search,
message: message,
user_input: input,
confidence: confidence,
cwe_id: [862],
link: 'https://positive.security/blog/ransack-data-exfiltration'
end
end
end

def ransackable_allow_list? class_name
tracker.find_method(:ransackable_attributes, class_name, :class) and
tracker.find_method(:ransackable_associations, class_name, :class)
end
end
1 change: 1 addition & 0 deletions lib/brakeman/warning_codes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ module Brakeman::WarningCodes
:insecure_rsa_padding_mode => 126,
:missing_rsa_padding_mode => 127,
:small_rsa_key_size => 128,
:ransack_search => 129,

:custom_check => 9090,
}
Expand Down
2 changes: 2 additions & 0 deletions test/apps/rails7/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ]
# Reduces boot times through caching; required in config/boot.rb
gem "bootsnap", ">= 1.4.4", require: false

gem 'ransack', '~>3.0.0'

# Use Sass to process CSS
# gem "sassc-rails", "~> 2.1"

Expand Down
5 changes: 5 additions & 0 deletions test/apps/rails7/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ GEM
thor (~> 1.0)
zeitwerk (~> 2.5)
rake (13.0.6)
ransack (3.0.1)
activerecord (>= 6.0.4)
activesupport (>= 6.0.4)
i18n
regexp_parser (2.2.0)
reline (0.2.7)
io-console (~> 0.5)
Expand Down Expand Up @@ -217,6 +221,7 @@ DEPENDENCIES
jbuilder (~> 2.11)
puma (~> 5.0)
rails (~> 7.0.0)
ransack (~> 3.0.0)
selenium-webdriver (>= 4.0.0)
sprockets-rails (>= 3.4.1)
sqlite3 (~> 1.4)
Expand Down
6 changes: 6 additions & 0 deletions test/apps/rails7/app/controllers/admin_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AdminController < ApplicationController
def search_users
# Medium warning because it's probably an admin interface
User.ransack(params[:q])
end
end
12 changes: 12 additions & 0 deletions test/apps/rails7/app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ def redirect_back_or_to_with_fallback_disallow_host
redirect_back_or_to params[:x], allow_other_host: false # no warning
end

def search
User.ransack(params[:q])
end

def search_books
# Should not warn - search limited appropriately
Book.ransack(params[:q])

# Low confidence because no idea what `some_book` is
some_book.things.ransack(params[:q])
end

class << self
def just_here_for_test_coverage_thanks
end
Expand Down
5 changes: 5 additions & 0 deletions test/apps/rails7/app/models/book.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Book < Thing
def self.ransackable_attributes(auth_object = nil)
[:author, :title]
end
end
7 changes: 7 additions & 0 deletions test/apps/rails7/app/models/thing.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class Thing < ApplicationRecord
class << self
def ransackable_associations(auth_object = nil)
[]
end
end
end
57 changes: 55 additions & 2 deletions test/tests/rails7.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def expected
:controller => 0,
:model => 0,
:template => 0,
:warning => 23
:warning => 26
}
end

Expand All @@ -26,7 +26,7 @@ def test_ruby_2_7_eol
warning_code: 123,
fingerprint: "425dcb3af9624f11f12d777d6f9fe05995719975a155c30012baa6b9dc3487df",
warning_type: "Unmaintained Dependency",
line: 230,
line: 235,
message: /^Support\ for\ Ruby\ 2\.7\.0\ ends\ on\ 2023\-03\-3/,
confidence: 2,
relative_path: "Gemfile.lock",
Expand Down Expand Up @@ -409,4 +409,57 @@ def test_redirect_back_or_to
code: s(:call, nil, :redirect_back_or_to, s(:call, s(:params), :[], s(:lit, :x))),
user_input: s(:call, s(:params), :[], s(:lit, :x))
end

def test_missing_authorization_ransack
assert_warning check_name: "Ransack",
type: :warning,
warning_code: 129,
fingerprint: "ae28bfeb8423952ffae97149292175b2d10c36c4904ee198ab7b2eda4e05c3e0",
warning_type: "Missing Authorization",
line: 41,
message: /^Unrestricted\ search\ using\ `ransack`\ libr/,
confidence: 0,
relative_path: "app/controllers/users_controller.rb",
code: s(:call, s(:const, :User), :ransack, s(:call, s(:params), :[], s(:lit, :q))),
user_input: s(:call, s(:params), :[], s(:lit, :q))
end

def test_missing_authorization_ransack_admin
assert_warning check_name: "Ransack",
type: :warning,
warning_code: 129,
fingerprint: "cb03f7424bc739b26e3789e2d9bb6893b4f2f517dbe10d4d3f3f19b4cf845459",
warning_type: "Missing Authorization",
line: 4,
message: /^Unrestricted\ search\ using\ `ransack`\ libr/,
confidence: 1,
relative_path: "app/controllers/admin_controller.rb",
code: s(:call, s(:const, :User), :ransack, s(:call, s(:params), :[], s(:lit, :q))),
user_input: s(:call, s(:params), :[], s(:lit, :q))
end

def test_missing_authorization_ransack_2
assert_no_warning check_name: "Ransack",
type: :warning,
warning_code: 129,
warning_type: "Missing Authorization",
line: 46,
message: /^Unrestricted\ search\ using\ `ransack`\ libr/,
confidence: 0,
relative_path: "app/controllers/users_controller.rb"
end

def test_missing_authorization_ransack_low
assert_warning check_name: "Ransack",
type: :warning,
warning_code: 129,
fingerprint: "50e236d8fbc9db0f67e0011941b92b08d0ece176ce4b8caea89d372f007a4873",
warning_type: "Missing Authorization",
line: 49,
message: /^Unrestricted\ search\ using\ `ransack`\ libr/,
confidence: 2,
relative_path: "app/controllers/users_controller.rb",
code: s(:call, s(:call, s(:call, nil, :some_book), :things), :ransack, s(:call, s(:params), :[], s(:lit, :q))),
user_input: s(:call, s(:params), :[], s(:lit, :q))
end
end

0 comments on commit 9f6eb56

Please sign in to comment.