Prevent string polymorphic route arguments
url_for supports building polymorphic URLs via an array of arguments (usually symbols and records). If an array is passed, strings can result in unwanted route helper calls. CVE-2021-22885
This commit is contained in:
parent
b61b94181b
commit
c4c21a9f8d
|
@ -287,10 +287,12 @@ module ActionDispatch
|
||||||
|
|
||||||
args = []
|
args = []
|
||||||
|
|
||||||
route = record_list.map { |parent|
|
route = record_list.map do |parent|
|
||||||
case parent
|
case parent
|
||||||
when Symbol, String
|
when Symbol
|
||||||
parent.to_s
|
parent.to_s
|
||||||
|
when String
|
||||||
|
raise(ArgumentError, "Please use symbols for polymorphic route arguments.")
|
||||||
when Class
|
when Class
|
||||||
args << parent
|
args << parent
|
||||||
parent.model_name.singular_route_key
|
parent.model_name.singular_route_key
|
||||||
|
@ -298,12 +300,14 @@ module ActionDispatch
|
||||||
args << parent.to_model
|
args << parent.to_model
|
||||||
parent.to_model.model_name.singular_route_key
|
parent.to_model.model_name.singular_route_key
|
||||||
end
|
end
|
||||||
}
|
end
|
||||||
|
|
||||||
route <<
|
route <<
|
||||||
case record
|
case record
|
||||||
when Symbol, String
|
when Symbol
|
||||||
record.to_s
|
record.to_s
|
||||||
|
when String
|
||||||
|
raise(ArgumentError, "Please use symbols for polymorphic route arguments.")
|
||||||
when Class
|
when Class
|
||||||
@key_strategy.call record.model_name
|
@key_strategy.call record.model_name
|
||||||
else
|
else
|
||||||
|
|
|
@ -130,6 +130,14 @@ class RedirectController < ActionController::Base
|
||||||
redirect_to nil
|
redirect_to nil
|
||||||
end
|
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
|
def redirect_to_params
|
||||||
redirect_to ActionController::Parameters.new(status: 200, protocol: "javascript", f: "%0Aeval(name)")
|
redirect_to ActionController::Parameters.new(status: 200, protocol: "javascript", f: "%0Aeval(name)")
|
||||||
end
|
end
|
||||||
|
@ -376,6 +384,43 @@ class RedirectTest < ActionController::TestCase
|
||||||
end
|
end
|
||||||
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
|
def test_redirect_to_nil
|
||||||
error = assert_raise(ActionController::ActionControllerError) do
|
error = assert_raise(ActionController::ActionControllerError) do
|
||||||
get :redirect_to_nil
|
get :redirect_to_nil
|
||||||
|
|
|
@ -464,12 +464,6 @@ class PolymorphicRoutesTest < ActionController::TestCase
|
||||||
end
|
end
|
||||||
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
|
def test_with_array_containing_symbols
|
||||||
with_test_routes do
|
with_test_routes do
|
||||||
assert_url "http://example.com/series/new", [:new, :series]
|
assert_url "http://example.com/series/new", [:new, :series]
|
||||||
|
@ -624,6 +618,22 @@ class PolymorphicRoutesTest < ActionController::TestCase
|
||||||
end
|
end
|
||||||
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)
|
def with_namespaced_routes(name)
|
||||||
with_routing do |set|
|
with_routing do |set|
|
||||||
set.draw do
|
set.draw do
|
||||||
|
|
Loading…
Reference in New Issue