Merge branch 'main-sec'

* main-sec:
  Prevent slow regex when parsing host authorization header
  Escape allow list hosts correctly
  Prevent string polymorphic route arguments
  Prevent catastrophic backtracking during mime parsing
This commit is contained in:
Aaron Patterson 2021-05-05 09:03:28 -07:00
commit 908d7bc1bf
No known key found for this signature in database
GPG Key ID: 953170BCB4FFAFC6
9 changed files with 99 additions and 13 deletions

View File

@ -407,7 +407,7 @@ module ActionController
module Token
TOKEN_KEY = "token="
TOKEN_REGEX = /^(Token|Bearer)\s+/
AUTHN_PAIR_DELIMITERS = /(?:,|;|\t+)/
AUTHN_PAIR_DELIMITERS = /(?:,|;|\t)/
extend self
module ControllerMethods

View File

@ -228,7 +228,7 @@ module Mime
MIME_PARAMETER_KEY = "[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}"
MIME_PARAMETER_VALUE = "#{Regexp.escape('"')}?[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}#{Regexp.escape('"')}?"
MIME_PARAMETER = "\s*\;\s*#{MIME_PARAMETER_KEY}(?:\=#{MIME_PARAMETER_VALUE})?"
MIME_REGEXP = /\A(?:\*\/\*|#{MIME_NAME}\/(?:\*|#{MIME_NAME})(?:\s*#{MIME_PARAMETER}\s*)*)\z/
MIME_REGEXP = /\A(?:\*\/\*|#{MIME_NAME}\/(?:\*|#{MIME_NAME})(?>\s*#{MIME_PARAMETER}\s*)*)\z/
class InvalidMimeType < StandardError; end

View File

@ -53,7 +53,7 @@ module ActionDispatch
if host.start_with?(".")
/\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/i
else
/\A#{host}\z/i
/\A#{Regexp.escape host}\z/i
end
end
end

View File

@ -287,10 +287,12 @@ module ActionDispatch
args = []
route = record_list.map { |parent|
route = record_list.map do |parent|
case parent
when Symbol, String
when Symbol
parent.to_s
when String
raise(ArgumentError, "Please use symbols for polymorphic route arguments.")
when Class
args << parent
parent.model_name.singular_route_key
@ -298,12 +300,14 @@ module ActionDispatch
args << parent.to_model
parent.to_model.model_name.singular_route_key
end
}
end
route <<
case record
when Symbol, String
when Symbol
record.to_s
when String
raise(ArgumentError, "Please use symbols for polymorphic route arguments.")
when Class
@key_strategy.call record.model_name
else

View File

@ -88,6 +88,16 @@ class HttpTokenAuthenticationTest < ActionController::TestCase
assert_equal "HTTP Token: Access denied.\n", @response.body, "Authentication header was not properly parsed"
end
test "authentication request with evil header" do
@request.env["HTTP_AUTHORIZATION"] = "Token ." + " " * (1024*80-8) + "."
Timeout.timeout(1) do
get :index
end
assert_response :unauthorized
assert_equal "HTTP Token: Access denied.\n", @response.body, "Authentication header was not properly parsed"
end
test "successful authentication request with Bearer instead of Token" do
@request.env["HTTP_AUTHORIZATION"] = "Bearer lifo"
get :index

View File

@ -130,6 +130,14 @@ class RedirectController < ActionController::Base
redirect_to nil
end
def redirect_to_polymorphic
redirect_to [:internal, Workshop.new(5)]
end
def redirect_to_polymorphic_string_args
redirect_to ["internal", Workshop.new(5)]
end
def redirect_to_params
redirect_to ActionController::Parameters.new(status: 200, protocol: "javascript", f: "%0Aeval(name)")
end
@ -376,6 +384,43 @@ class RedirectTest < ActionController::TestCase
end
end
def test_polymorphic_redirect
with_routing do |set|
set.draw do
namespace :internal do
resources :workshops
end
ActiveSupport::Deprecation.silence do
get ":controller/:action"
end
end
get :redirect_to_polymorphic
assert_equal "http://test.host/internal/workshops/5", redirect_to_url
assert_redirected_to [:internal, Workshop.new(5)]
end
end
def test_polymorphic_redirect_with_string_args
with_routing do |set|
set.draw do
namespace :internal do
resources :workshops
end
ActiveSupport::Deprecation.silence do
get ":controller/:action"
end
end
error = assert_raises(ArgumentError) do
get :redirect_to_polymorphic_string_args
end
assert_equal("Please use symbols for polymorphic route arguments.", error.message)
end
end
def test_redirect_to_nil
error = assert_raise(ActionController::ActionControllerError) do
get :redirect_to_nil

View File

@ -232,6 +232,17 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
assert_match "Blocked host: attacker.com#x.example.com", response.body
end
test "blocks requests to similar host" do
@app = ActionDispatch::HostAuthorization.new(App, "sub.example.com")
get "/", env: {
"HOST" => "sub-example.com",
}
assert_response :forbidden
assert_match "Blocked host: sub-example.com", response.body
end
test "config setting action_dispatch.hosts_response_app is deprecated" do
assert_deprecated do
ActionDispatch::HostAuthorization.new(App, "example.com", ->(env) { true })

View File

@ -231,6 +231,12 @@ class MimeTypeTest < ActiveSupport::TestCase
assert_raises Mime::Type::InvalidMimeType do
Mime::Type.new(nil)
end
assert_raises Mime::Type::InvalidMimeType do
Timeout.timeout(1) do # Shouldn't take more than 1s
Mime::Type.new("text/html ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0 ;0;")
end
end
end
test "holds a reference to mime symbols" do

View File

@ -464,12 +464,6 @@ class PolymorphicRoutesTest < ActionController::TestCase
end
end
def test_with_array_containing_single_string_name
with_test_routes do
assert_url "http://example.com/projects", ["projects"]
end
end
def test_with_array_containing_symbols
with_test_routes do
assert_url "http://example.com/series/new", [:new, :series]
@ -624,6 +618,22 @@ class PolymorphicRoutesTest < ActionController::TestCase
end
end
def test_string_route_arguments
with_admin_test_routes do
error = assert_raises(ArgumentError) do
polymorphic_url(["admin", @project])
end
assert_equal("Please use symbols for polymorphic route arguments.", error.message)
error = assert_raises(ArgumentError) do
polymorphic_url([@project, "bid"])
end
assert_equal("Please use symbols for polymorphic route arguments.", error.message)
end
end
def with_namespaced_routes(name)
with_routing do |set|
set.draw do