1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Merge pull request #33718 from kddeisz/permit-list

Finish converting whitelist and blacklist references
This commit is contained in:
Matthew Draper 2018-08-29 14:07:37 +09:30 committed by GitHub
commit 068fe7dc90
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 63 additions and 52 deletions

View file

@ -5,8 +5,8 @@ require "active_support/core_ext/hash/slice"
module ActionController
# This module is deprecated in favor of +config.force_ssl+ in your environment
# config file. This will ensure all communication to non-whitelisted endpoints
# served by your application occurs over HTTPS.
# config file. This will ensure all endpoints not explicitly marked otherwise
# will have all communication served over HTTPS.
module ForceSSL # :nodoc:
extend ActiveSupport::Concern
include AbstractController::Callbacks

View file

@ -86,7 +86,7 @@ module ActionController
# Note: SSEs are not currently supported by IE. However, they are supported
# by Chrome, Firefox, Opera, and Safari.
class SSE
WHITELISTED_OPTIONS = %w( retry event id )
PERMITTED_OPTIONS = %w( retry event id )
def initialize(stream, options = {})
@stream = stream
@ -111,7 +111,7 @@ module ActionController
def perform_write(json, options)
current_options = @options.merge(options).stringify_keys
WHITELISTED_OPTIONS.each do |option_name|
PERMITTED_OPTIONS.each do |option_name|
if (option_value = current_options[option_name])
@stream.write "#{option_name}: #{option_value}\n"
end

View file

@ -11,7 +11,7 @@ module ActionController #:nodoc:
# @people = Person.all
# end
#
# That action implicitly responds to all formats, but formats can also be whitelisted:
# That action implicitly responds to all formats, but formats can also be explicitly enumerated:
#
# def index
# @people = Person.all

View file

@ -45,7 +45,7 @@ module ActionController #:nodoc:
# the same origin. Note however that any cross-origin third party domain
# allowed via {CORS}[https://en.wikipedia.org/wiki/Cross-origin_resource_sharing]
# will also be able to create XHR requests. Be sure to check your
# CORS whitelist before disabling forgery protection for XHR.
# CORS configuration before disabling forgery protection for XHR.
#
# CSRF protection is turned on with the <tt>protect_from_forgery</tt> method.
# By default <tt>protect_from_forgery</tt> protects your session with

View file

@ -58,7 +58,7 @@ module ActionController
# == Action Controller \Parameters
#
# Allows you to choose which attributes should be whitelisted for mass updating
# Allows you to choose which attributes should be permitted for mass updating
# and thus prevent accidentally exposing that which shouldn't be exposed.
# Provides two methods for this purpose: #require and #permit. The former is
# used to mark parameters as required. The latter is used to set the parameter
@ -505,7 +505,7 @@ module ActionController
#
# Note that if you use +permit+ in a key that points to a hash,
# it won't allow all the hash. You also need to specify which
# attributes inside the hash should be whitelisted.
# attributes inside the hash should be permitted.
#
# params = ActionController::Parameters.new({
# person: {
@ -997,8 +997,8 @@ module ActionController
#
# It provides an interface for protecting attributes from end-user
# assignment. This makes Action Controller parameters forbidden
# to be used in Active Model mass assignment until they have been
# whitelisted.
# to be used in Active Model mass assignment until they have been explicitly
# enumerated.
#
# In addition, parameters can be marked as required and flow through a
# predefined raise/rescue flow to end up as a <tt>400 Bad Request</tt> with no
@ -1034,7 +1034,7 @@ module ActionController
# end
#
# In order to use <tt>accepts_nested_attributes_for</tt> with Strong \Parameters, you
# will need to specify which nested attributes should be whitelisted. You might want
# will need to specify which nested attributes should be permitted. You might want
# to allow +:id+ and +:_destroy+, see ActiveRecord::NestedAttributes for more information.
#
# class Person
@ -1052,7 +1052,7 @@ module ActionController
# private
#
# def person_params
# # It's mandatory to specify the nested attributes that should be whitelisted.
# # It's mandatory to specify the nested attributes that should be permitted.
# # If you use `permit` with just the key that points to the nested attributes hash,
# # it will return an empty hash.
# params.require(:person).permit(:name, :age, pets_attributes: [ :id, :name, :category ])

View file

@ -85,10 +85,7 @@ module ActionDispatch
if variant.all? { |v| v.is_a?(Symbol) }
@variant = ActiveSupport::ArrayInquirer.new(variant)
else
raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols. " \
"For security reasons, never directly set the variant to a user-provided value, " \
"like params[:variant].to_sym. Check user-provided value against a whitelist first, " \
"then set the variant: request.variant = :tablet if params[:variant] == 'tablet'"
raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols."
end
end

View file

@ -553,10 +553,10 @@ module ActionDispatch
#
# match 'json_only', constraints: { format: 'json' }, via: :get
#
# class Whitelist
# class PermitList
# def matches?(request) request.remote_ip == '1.2.3.4' end
# end
# match 'path', to: 'c#a', constraints: Whitelist.new, via: :get
# match 'path', to: 'c#a', constraints: PermitList.new, via: :get
#
# See <tt>Scoping#constraints</tt> for more examples with its scope
# equivalent.

View file

@ -20,7 +20,7 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase
end
end
test "permits parameters that are whitelisted" do
test "allows both explicitly listed and always-permitted parameters" do
params = ActionController::Parameters.new(
book: { pages: 65 },
format: "json")

View file

@ -10,7 +10,7 @@ module ActionView
# These helper methods extend Action View making them callable within your template files.
module SanitizeHelper
extend ActiveSupport::Concern
# Sanitizes HTML input, stripping all tags and attributes that aren't whitelisted.
# Sanitizes HTML input, stripping all but known-safe tags and attributes.
#
# It also strips href/src attributes with unsafe protocols like
# <tt>javascript:</tt>, while also protecting against attempts to use Unicode,
@ -40,7 +40,7 @@ module ActionView
#
# <%= sanitize @comment.body %>
#
# Providing custom whitelisted tags and attributes:
# Providing custom lists of permitted tags and attributes:
#
# <%= sanitize @comment.body, tags: %w(strong em a), attributes: %w(href) %>
#

View file

@ -14,7 +14,17 @@ module ActionView
class_attribute :erb_implementation, default: Erubi
# Do not escape templates of these mime types.
class_attribute :escape_whitelist, default: ["text/plain"]
class_attribute :escape_ignore_list, default: ["text/plain"]
[self, singleton_class].each do |base|
base.send(:alias_method, :escape_whitelist, :escape_ignore_list)
base.send(:alias_method, :escape_whitelist=, :escape_ignore_list=)
base.deprecate(
escape_whitelist: "use #escape_ignore_list instead",
:escape_whitelist= => "use #escape_ignore_list= instead"
)
end
ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*")
@ -47,7 +57,7 @@ module ActionView
self.class.erb_implementation.new(
erb,
escape: (self.class.escape_whitelist.include? template.type),
escape: (self.class.escape_ignore_list.include? template.type),
trim: (self.class.erb_trim_mode == "-")
).src
end

View file

@ -24,18 +24,20 @@ module ActiveJob
module Arguments
extend self
# :nodoc:
TYPE_WHITELIST = [ NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass ]
PERMITTED_TYPES = [ NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass ]
# Serializes a set of arguments. Whitelisted types are returned
# as-is. Arrays/Hashes are serialized element by element.
# All other types are serialized using GlobalID.
# Serializes a set of arguments. Intrinsic types that can safely be
# serialized without mutation are returned as-is. Arrays/Hashes are
# serialized element by element. All other types are serialized using
# GlobalID.
def serialize(arguments)
arguments.map { |argument| serialize_argument(argument) }
end
# Deserializes a set of arguments. Whitelisted types are returned
# as-is. Arrays/Hashes are deserialized element by element.
# All other types are deserialized using GlobalID.
# Deserializes a set of arguments. Instrinsic types that can safely be
# deserialized without mutation are returned as-is. Arrays/Hashes are
# deserialized element by element. All other types are deserialized using
# GlobalID.
def deserialize(arguments)
arguments.map { |argument| deserialize_argument(argument) }
rescue
@ -64,7 +66,7 @@ module ActiveJob
def serialize_argument(argument)
case argument
when *TYPE_WHITELIST
when *PERMITTED_TYPES
argument
when GlobalID::Identification
convert_to_global_id_hash(argument)
@ -88,7 +90,7 @@ module ActiveJob
case argument
when String
GlobalID::Locator.locate(argument) || argument
when *TYPE_WHITELIST
when *PERMITTED_TYPES
argument
when Array
argument.map { |arg| deserialize_argument(arg) }

View file

@ -31,7 +31,7 @@ module ActiveRecord
end
}
BLACKLISTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass)
RESTRICTED_CLASS_METHODS = %w(private public protected allocate new name parent superclass)
class GeneratedAttributeMethods < Module #:nodoc:
include Mutex_m
@ -123,7 +123,7 @@ module ActiveRecord
# A class method is 'dangerous' if it is already (re)defined by Active Record, but
# not by any ancestors. (So 'puts' is not dangerous but 'new' is.)
def dangerous_class_method?(method_name)
BLACKLISTED_CLASS_METHODS.include?(method_name.to_s) || class_method_defined_within?(method_name, Base)
RESTRICTED_CLASS_METHODS.include?(method_name.to_s) || class_method_defined_within?(method_name, Base)
end
def class_method_defined_within?(name, klass, superklass = klass.superclass) # :nodoc:
@ -167,12 +167,14 @@ module ActiveRecord
end
end
# Regexp whitelist. Matches the following:
# Regexp for column names (with or without a table name prefix). Matches
# the following:
# "#{table_name}.#{column_name}"
# "#{column_name}"
COLUMN_NAME_WHITELIST = /\A(?:\w+\.)?\w+\z/i
COLUMN_NAME = /\A(?:\w+\.)?\w+\z/i
# Regexp whitelist. Matches the following:
# Regexp for column names with order (with or without a table name
# prefix, with or without various order modifiers). Matches the following:
# "#{table_name}.#{column_name}"
# "#{table_name}.#{column_name} #{direction}"
# "#{table_name}.#{column_name} #{direction} NULLS FIRST"
@ -181,7 +183,7 @@ module ActiveRecord
# "#{column_name} #{direction}"
# "#{column_name} #{direction} NULLS FIRST"
# "#{column_name} NULLS LAST"
COLUMN_NAME_ORDER_WHITELIST = /
COLUMN_NAME_WITH_ORDER = /
\A
(?:\w+\.)?
\w+
@ -190,12 +192,12 @@ module ActiveRecord
\z
/ix
def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc:
def disallow_raw_sql!(args, permit: COLUMN_NAME) # :nodoc:
unexpected = args.reject do |arg|
arg.kind_of?(Arel::Node) ||
arg.is_a?(Arel::Nodes::SqlLiteral) ||
arg.is_a?(Arel::Attributes::Attribute) ||
arg.to_s.split(/\s*,\s*/).all? { |part| whitelist.match?(part) }
arg.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) }
end
return if unexpected.none?

View file

@ -190,7 +190,7 @@ module ActiveRecord
relation = apply_join_dependency
relation.pluck(*column_names)
else
enforce_raw_sql_whitelist(column_names)
disallow_raw_sql!(column_names)
relation = spawn
relation.select_values = column_names.map { |cn|
@klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn

View file

@ -1133,9 +1133,9 @@ module ActiveRecord
end
order_args.flatten!
@klass.enforce_raw_sql_whitelist(
@klass.disallow_raw_sql!(
order_args.flat_map { |a| a.is_a?(Hash) ? a.keys : a },
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
permit: AttributeMethods::ClassMethods::COLUMN_NAME_WITH_ORDER
)
validate_order_args(order_args)

View file

@ -61,8 +61,8 @@ module ActiveRecord
# # => "id ASC"
def sanitize_sql_for_order(condition)
if condition.is_a?(Array) && condition.first.to_s.include?("?")
enforce_raw_sql_whitelist([condition.first],
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
disallow_raw_sql!([condition.first],
permit: AttributeMethods::ClassMethods::COLUMN_NAME_WITH_ORDER
)
# Ensure we aren't dealing with a subclass of String that might

View file

@ -40,7 +40,7 @@ if ActiveRecord::Base.connection.supports_explain?
assert_equal binds, queries[0][1]
end
def test_collects_nothing_if_the_statement_is_not_whitelisted
def test_collects_nothing_if_the_statement_is_not_explainable
SUBSCRIBER.finish(nil, nil, name: "SQL", sql: "SHOW max_identifier_length")
assert_empty queries
end

View file

@ -5,7 +5,7 @@ require "models/post"
require "models/comment"
module ActiveRecord
module DelegationWhitelistTests
module ArrayDelegationTests
ARRAY_DELEGATES = [
:+, :-, :|, :&, :[], :shuffle,
:all?, :collect, :compact, :detect, :each, :each_cons, :each_with_index,
@ -38,7 +38,7 @@ module ActiveRecord
end
class DelegationAssociationTest < ActiveRecord::TestCase
include DelegationWhitelistTests
include ArrayDelegationTests
include DeprecatedArelDelegationTests
def target
@ -47,7 +47,7 @@ module ActiveRecord
end
class DelegationRelationTest < ActiveRecord::TestCase
include DelegationWhitelistTests
include ArrayDelegationTests
include DeprecatedArelDelegationTests
def target

View file

@ -324,7 +324,7 @@ class FakeKlass
table[name]
end
def enforce_raw_sql_whitelist(*args)
def disallow_raw_sql!(*args)
# noop
end

View file

@ -51,12 +51,12 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
end
def test_add_migration_with_table_having_from_in_title
migration = "add_email_address_to_blacklisted_from_campaign"
migration = "add_email_address_to_excluded_from_campaign"
run_generator [migration, "email_address:string"]
assert_migration "db/migrate/#{migration}.rb" do |content|
assert_method :change, content do |change|
assert_match(/add_column :blacklisted_from_campaigns, :email_address, :string/, change)
assert_match(/add_column :excluded_from_campaigns, :email_address, :string/, change)
end
end
end