From fa487763d98ccf9c3e66fdb44f09af5c37a50fe5 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Tue, 12 Apr 2016 02:41:06 +0530 Subject: Changed default behaviour of `ActiveSupport::SecurityUtils.secure_compare`, to make it not leak length information even for variable length string. Renamed old `ActiveSupport::SecurityUtils.secure_compare` to `fixed_length_secure_compare`, and started raising `ArgumentError` in case of length mismatch of passed strings. --- actionpack/lib/action_controller/metal/http_authentication.rb | 11 ++++------- .../lib/action_controller/metal/request_forgery_protection.rb | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index d8bc895265..09df39db1f 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -70,10 +70,10 @@ module ActionController before_action(options.except(:name, :password, :realm)) do authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password| # This comparison uses & so that it doesn't short circuit and - # uses `variable_size_secure_compare` so that length information + # uses `secure_compare` so that length information # isn't leaked. - ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) & - ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password]) + ActiveSupport::SecurityUtils.secure_compare(name, options[:name]) & + ActiveSupport::SecurityUtils.secure_compare(password, options[:password]) end end end @@ -348,10 +348,7 @@ module ActionController # authenticate_or_request_with_http_token do |token, options| # # Compare the tokens in a time-constant manner, to mitigate # # timing attacks. - # ActiveSupport::SecurityUtils.secure_compare( - # ::Digest::SHA256.hexdigest(token), - # ::Digest::SHA256.hexdigest(TOKEN) - # ) + # ActiveSupport::SecurityUtils.secure_compare(token, TOKEN) # end # end # end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 5051c02a62..13662fc021 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -353,7 +353,7 @@ module ActionController #:nodoc: end def compare_with_real_token(token, session) # :doc: - ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session)) end def valid_per_form_csrf_token?(token, session) # :doc: @@ -364,7 +364,7 @@ module ActionController #:nodoc: request.request_method ) - ActiveSupport::SecurityUtils.secure_compare(token, correct_token) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, correct_token) else false end -- cgit v1.2.3 From 5349f231089ddb884c7cb5405e74b7871383d65d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sat, 30 Sep 2017 18:33:53 +0900 Subject: Remove `:api:` tag that has leaked on the doc directly [ci skip] Currently `:api:` tag has leaked on the doc directly since RDoc doesn't support `:api:` tag directive. http://api.rubyonrails.org/v5.1/classes/AbstractController/Rendering.html So `:api: private` doesn't work as expected. We are using `:nodoc:` for the purpose. Related #13989. --- actionpack/lib/action_controller/metal/instrumentation.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 476f0843b2..5ef83af07a 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -83,15 +83,12 @@ module ActionController # def cleanup_view_runtime # super - time_taken_in_something_expensive # end - # - # :api: plugin def cleanup_view_runtime yield end # Every time after an action is processed, this method is invoked # with the payload, so you can add more information. - # :api: plugin def append_info_to_payload(payload) payload[:view_runtime] = view_runtime end @@ -100,7 +97,6 @@ module ActionController # A hook which allows other frameworks to log what happened during # controller process action. This method should return an array # with the messages to be added. - # :api: plugin def log_process_action(payload) #:nodoc: messages, view_runtime = [], payload[:view_runtime] messages << ("Views: %.1fms" % view_runtime.to_f) if view_runtime -- cgit v1.2.3 From 0db6a14ae16b143e078375ff7f3c940cf707290b Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Tue, 10 Oct 2017 14:15:56 +0300 Subject: Add allow_other_host option to redirect_back method --- .../lib/action_controller/metal/redirecting.rb | 23 +++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 5cd8568d8d..b8a80eef31 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -79,15 +79,18 @@ module ActionController # redirect_back fallback_location: "/images/screenshot.jpg" # redirect_back fallback_location: posts_url # redirect_back fallback_location: proc { edit_post_url(@post) } + # redirect_back fallback_location: '/', allow_other_host: false # - # All options that can be passed to redirect_to are accepted as + # ==== Options + # * :fallback_location - The default fallback location that will be used on missing `Referer` header. + # * :allow_other_host - Allows or dissallow redirection to the host that is different to the current host + # + # All other options that can be passed to redirect_to are accepted as # options and the behavior is identical. - def redirect_back(fallback_location:, **args) - if referer = request.headers["Referer"] - redirect_to referer, **args - else - redirect_to fallback_location, **args - end + def redirect_back(fallback_location:, allow_other_host: true, **args) + referer = request.headers["Referer"] + redirect_to_referer = referer && (allow_other_host || _url_host_allowed?(referer)) + redirect_to redirect_to_referer ? referer : fallback_location, **args end def _compute_redirect_to_location(request, options) #:nodoc: @@ -120,5 +123,11 @@ module ActionController 302 end end + + def _url_host_allowed?(url) + URI(url.to_s).host == request.host + rescue ArgumentError, URI::Error + false + end end end -- cgit v1.2.3 From b1ee5e8e7f98b32196423a144e7a87ae284c7a5c Mon Sep 17 00:00:00 2001 From: Mike Boone Date: Tue, 10 Oct 2017 23:08:03 -0400 Subject: Fix some typos. --- actionpack/lib/action_controller/metal/redirecting.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index b8a80eef31..8de57f9199 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -83,7 +83,7 @@ module ActionController # # ==== Options # * :fallback_location - The default fallback location that will be used on missing `Referer` header. - # * :allow_other_host - Allows or dissallow redirection to the host that is different to the current host + # * :allow_other_host - Allows or disallow redirection to the host that is different to the current host # # All other options that can be passed to redirect_to are accepted as # options and the behavior is identical. -- cgit v1.2.3 From 879d540adc34603f0fd1ac1a44763598e9ccc551 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sun, 15 Oct 2017 16:01:02 +0900 Subject: Remove unused `before_filters` This method added by 1008511. It is unnecessary because it is no longer called by 19c3495. --- actionpack/lib/action_controller/metal/testing.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/testing.rb b/actionpack/lib/action_controller/metal/testing.rb index b07f1f3d8c..6e8a95040f 100644 --- a/actionpack/lib/action_controller/metal/testing.rb +++ b/actionpack/lib/action_controller/metal/testing.rb @@ -12,11 +12,5 @@ module ActionController self.params = nil end end - - module ClassMethods - def before_filters - _process_action_callbacks.find_all { |x| x.kind == :before }.map(&:name) - end - end end end -- cgit v1.2.3 From b1c8610fca6d8a59246e190bfaed1aa445480b07 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Wed, 18 Oct 2017 16:28:29 +0900 Subject: Remove unused `UnknownController` class `UnknownController` was added in b1999be, but it is not used anywhere. --- actionpack/lib/action_controller/metal/exceptions.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index f808295720..a65857d6ef 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -34,9 +34,6 @@ module ActionController class NotImplemented < MethodNotAllowed #:nodoc: end - class UnknownController < ActionControllerError #:nodoc: - end - class MissingFile < ActionControllerError #:nodoc: end -- cgit v1.2.3 From a52ff1b0848e8bb2310738ec7b762b82edbbfcb2 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 20 Oct 2017 05:34:47 +0900 Subject: Keep `:api: plugin` methods in the doc [ci skip] `:api:` tag was removed in 5349f231 since RDoc doesn't support `:api:` tag. But those methods are not private API, they are public API for renderers. The renderers should be able to know that they can override this method. --- actionpack/lib/action_controller/metal/instrumentation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 5ef83af07a..be9449629f 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -83,13 +83,13 @@ module ActionController # def cleanup_view_runtime # super - time_taken_in_something_expensive # end - def cleanup_view_runtime + def cleanup_view_runtime # :doc: yield end # Every time after an action is processed, this method is invoked # with the payload, so you can add more information. - def append_info_to_payload(payload) + def append_info_to_payload(payload) # :doc: payload[:view_runtime] = view_runtime end -- cgit v1.2.3 From b0d0c9f40df3bee73d3f36005238d7d0227579d2 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sat, 21 Oct 2017 22:18:17 +0900 Subject: [Action Pack] require => require_relative This basically reverts e9fca7668b9eba82bcc832cb0061459703368397, d08da958b9ae17d4bbe4c9d7db497ece2450db5f, d1fe1dcf8ab1c0210a37c2a78c1ee52cf199a66d, and 68eaf7b4d5f2bb56d939f71c5ece2d61cf6680a3 --- actionpack/lib/action_controller/metal/data_streaming.rb | 2 +- actionpack/lib/action_controller/metal/request_forgery_protection.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 882f6f3d0a..5a82ccf668 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative "exceptions" +require "action_controller/metal/exceptions" module ActionController #:nodoc: # Methods for sending arbitrary data and for streaming files to the browser, diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 813a7e00d4..d6cd5fd9e0 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "rack/session/abstract/id" -require_relative "exceptions" +require "action_controller/metal/exceptions" require "active_support/security_utils" module ActionController #:nodoc: -- cgit v1.2.3 From 7b51b140d5da80402a114c06f30fbf66e12c676e Mon Sep 17 00:00:00 2001 From: Ryan Perez Date: Sun, 8 Mar 2015 01:36:29 +0000 Subject: Fixed functionality to include method in params_wrapper.rb to properly wrap all attributes, including those which are nested. --- actionpack/lib/action_controller/metal/params_wrapper.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index f4f2381286..9f79d796cc 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -113,6 +113,13 @@ module ActionController self.include = m.attribute_names end end + if m.respond_to?(:nested_attributes_options) && m.nested_attributes_options.any? + nested_attributes_names = self.nested_attributes_options.keys.map do |key| + key.to_s.concat('_attributes').to_sym + end + self.include += nested_attributes_names + end + end end end -- cgit v1.2.3 From dd423a743e9d47aaa3d15e4f7dc0fc64157cd55c Mon Sep 17 00:00:00 2001 From: Kelton Manzanares Date: Fri, 20 Oct 2017 19:07:14 -0600 Subject: checking for nested attributes when attribute names specified to wrap them as well --- actionpack/lib/action_controller/metal/params_wrapper.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 9f79d796cc..a678377d4f 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -112,14 +112,15 @@ module ActionController else self.include = m.attribute_names end - end - if m.respond_to?(:nested_attributes_options) && m.nested_attributes_options.any? - nested_attributes_names = self.nested_attributes_options.keys.map do |key| - key.to_s.concat('_attributes').to_sym + + if m.respond_to?(:nested_attributes_options) && m.nested_attributes_options.keys.any? + self.include += m.nested_attributes_options.keys.map do |key| + key.to_s.concat("_attributes") + end end - self.include += nested_attributes_names - end + self.include + end end end end -- cgit v1.2.3 From acdba1c6a653bf5c787d3457af95b37708be1e2b Mon Sep 17 00:00:00 2001 From: Jack McCracken Date: Mon, 2 Oct 2017 16:35:13 -0400 Subject: Add a better error message when a "null" Origin header occurs --- .../lib/action_controller/metal/request_forgery_protection.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index d6cd5fd9e0..b2e6f86eeb 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -414,11 +414,21 @@ module ActionController #:nodoc: allow_forgery_protection end + NULL_ORIGIN_MESSAGE = <<-MSG.strip_heredoc + The browser returned a 'null' origin for a request with origin-based forgery protection turned on. This usually + means you have the 'no-referrer' Referrer-Policy header enabled, or that you the request came from a site that + refused to give its origin. This makes it impossible for Rails to verify the source of the requests. Likely the + best solution is to change your referrer policy to something less strict like same-origin or strict-same-origin. + If you cannot change the referrer policy, you can disable origin checking with the + Rails.application.config.action_controller.forgery_protection_origin_check setting. + MSG + # Checks if the request originated from the same origin by looking at the # Origin header. def valid_request_origin? # :doc: if forgery_protection_origin_check # We accept blank origin headers because some user agents don't send it. + raise InvalidAuthenticityToken, NULL_ORIGIN_MESSAGE if request.origin == "null" request.origin.nil? || request.origin == request.base_url else true -- cgit v1.2.3 From ab293293c33792621c90c02c5516a99e78460663 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sun, 5 Nov 2017 21:23:05 +0900 Subject: Show `RequestForgeryProtection` methods in api doc [ci skip] Several methods of `RequestForgeryProtection` are not showed in the api doc even though `:doc:` is specified. (e.g. `form_authenticity_param`) http://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection.html These methods are listed in the doc of v4.1. http://api.rubyonrails.org/v4.1/classes/ActionController/RequestForgeryProtection.html This is due to the influence of `:nodoc:` added in #18102, methods after `CROSS_ORIGIN_JAVASCRIPT_WARNING` not showed from the doc. Therefore, in order to show the method like originally, added `startdoc` after `CROSS_ORIGIN_JAVASCRIPT_WARNING`. --- actionpack/lib/action_controller/metal/request_forgery_protection.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index d6cd5fd9e0..bd133f24a1 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -248,6 +248,7 @@ module ActionController #:nodoc: "If you know what you're doing, go ahead and disable forgery " \ "protection on this action to permit cross-origin JavaScript embedding." private_constant :CROSS_ORIGIN_JAVASCRIPT_WARNING + # :startdoc: # If `verify_authenticity_token` was run (indicating that we have # forgery protection enabled for this request) then also verify that -- cgit v1.2.3 From 8c5115f95daa86cc9e79d6ad5c1076fee0f70903 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 10 Nov 2017 10:37:45 +0900 Subject: Bump RuboCop to 0.51.0 ## Summary RuboCop 0.51.0 was released. https://github.com/bbatsov/rubocop/releases/tag/v0.51.0 And rubocop-0-51 channel is available in Code Climate. https://github.com/codeclimate/codeclimate-rubocop/issues/109 This PR will bump RuboCop to 0.51.0 and fixes the following new offenses. ```console % bundle exec rubocop Inspecting 2358 files (snip) Offenses: actionpack/lib/action_controller/metal/http_authentication.rb:251:59: C: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. [key.strip, value.to_s.gsub(/^"|"$/, "").delete('\'')] ^^^^ activesupport/test/core_ext/load_error_test.rb:8:39: C: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. assert_raise(LoadError) { require 'no_this_file_don\'t_exist' } ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 2358 files inspected, 2 offenses detected ``` --- actionpack/lib/action_controller/metal/http_authentication.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 08d9b094f3..0c8132684a 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -248,7 +248,7 @@ module ActionController def decode_credentials(header) ActiveSupport::HashWithIndifferentAccess[header.to_s.gsub(/^Digest\s+/, "").split(",").map do |pair| key, value = pair.split("=", 2) - [key.strip, value.to_s.gsub(/^"|"$/, "").delete('\'')] + [key.strip, value.to_s.gsub(/^"|"$/, "").delete("'")] end] end -- cgit v1.2.3 From 3063ace1070e4ddb8d0cc09fbd23049e7b21617a Mon Sep 17 00:00:00 2001 From: "T.J. Schuck" Date: Wed, 22 Nov 2017 14:45:51 -0500 Subject: Update incorrect backtick usage in RDoc to teletype [ci skip] --- actionpack/lib/action_controller/metal/redirecting.rb | 4 ++-- .../lib/action_controller/metal/request_forgery_protection.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_controller/metal') diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 8de57f9199..87a2e29a3f 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -68,7 +68,7 @@ module ActionController # if possible, otherwise redirects to the provided default fallback # location. # - # The referrer information is pulled from the HTTP `Referer` (sic) header on + # The referrer information is pulled from the HTTP +Referer+ (sic) header on # the request. This is an optional header and its presence on the request is # subject to browser security settings and user preferences. If the request # is missing this header, the fallback_location will be used. @@ -82,7 +82,7 @@ module ActionController # redirect_back fallback_location: '/', allow_other_host: false # # ==== Options - # * :fallback_location - The default fallback location that will be used on missing `Referer` header. + # * :fallback_location - The default fallback location that will be used on missing +Referer+ header. # * :allow_other_host - Allows or disallow redirection to the host that is different to the current host # # All other options that can be passed to redirect_to are accepted as diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index bd133f24a1..906494ba16 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -216,7 +216,7 @@ module ActionController #:nodoc: # The actual before_action that is used to verify the CSRF token. # Don't override this directly. Provide your own forgery protection # strategy instead. If you override, you'll disable same-origin - # `