Skip to content

Commit 1a57c15

Browse files
committed
Handle path_params gracefully when a user sends ?path_params=string
In some cases, params are passed into url_for, causing path_params to be sent unintentionally. It is possible to both send a string, causing a 500 error, or send a hash: ?path_params[inject]=string. I spent some time before posting this reviewing whether it was possible to exploit the fact that path_params can be sent in query params, but I don't believe there to be a vulnerability. Although it is probably good practice to scrub this key when sending params to url_for just to be sure.
1 parent 2aa67ec commit 1a57c15

File tree

2 files changed

+11
-3
lines changed

2 files changed

+11
-3
lines changed

actionpack/lib/action_dispatch/journey/formatter.rb

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,13 @@ def message
6060

6161
def generate(name, options, path_parameters)
6262
original_options = options.dup
63-
path_params = options.delete(:path_params) || {}
64-
options = path_params.merge(options)
63+
path_params = options.delete(:path_params)
64+
if path_params.is_a?(Hash)
65+
options = path_params.merge(options)
66+
else
67+
path_params = nil
68+
options = options.dup
69+
end
6570
constraints = path_parameters.merge(options)
6671
missing_keys = nil
6772

@@ -79,7 +84,7 @@ def generate(name, options, path_parameters)
7984
# top-level params' normal behavior of generating query_params should be
8085
# preserved even if the same key is also a bind_param
8186
parameterized_parts.key?(key) || route.defaults.key?(key) ||
82-
(path_params.key?(key) && !original_options.key?(key))
87+
(path_params&.key?(key) && !original_options.key?(key))
8388
end
8489

8590
defaults = route.defaults

actionpack/test/controller/url_for_integration_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ def setup
8787
["/blog/2009", [ { controller: "posts", action: "show_date", year: 2009 }]],
8888
["/blog/2009/1", [ { controller: "posts", action: "show_date", year: 2009, month: 1 }]],
8989
["/blog/2009/1/1", [ { controller: "posts", action: "show_date", year: 2009, month: 1, day: 1 }]],
90+
["/blog/2009/1/1", [ { controller: "posts", action: "show_date", path_params: { year: 2009, month: 1, day: 1 } }]],
91+
["/blog/2009", [ { controller: "posts", action: "show_date", year: 2009, path_params: { year: 2024 } }]],
92+
["/blog/2009", [ { controller: "posts", action: "show_date", year: 2009, path_params: "ignores_a_string" }]],
9093

9194
["/archive/2010", [ { controller: "archive", action: "index", year: "2010" }]],
9295
["/archive", [ { controller: "archive", action: "index" }]],

0 commit comments

Comments
 (0)