Skip to content

Commit

Permalink
Prevent string polymorphic route arguments
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gmcgibbon authored and tenderlove committed May 4, 2021
1 parent b61b941 commit c4c21a9
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
12 changes: 8 additions & 4 deletions actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
Expand Up @@ -287,23 +287,27 @@ def handle_list(list)

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
else
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
Expand Down
45 changes: 45 additions & 0 deletions actionpack/test/controller/redirect_test.rb
Expand Up @@ -130,6 +130,14 @@ def redirect_to_nil
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
Expand Down Expand Up @@ -376,6 +384,43 @@ def test_redirect_to_record
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
Expand Down
22 changes: 16 additions & 6 deletions actionview/test/activerecord/polymorphic_routes_test.rb
Expand Up @@ -464,12 +464,6 @@ def test_with_array_containing_single_name
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]
Expand Down Expand Up @@ -624,6 +618,22 @@ def test_nested_routing_to_a_model_delegate
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
Expand Down

0 comments on commit c4c21a9

Please sign in to comment.