Skip to content

Commit

Permalink
Properly encode ID parameters to avoid possible information leak
Browse files Browse the repository at this point in the history
  • Loading branch information
tenderlove committed May 5, 2020
1 parent 326b452 commit 0de18f7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/active_resource/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ def element_path(id, prefix_options = {}, query_options = nil)
check_prefix_options(prefix_options)

prefix_options, query_options = split_options(prefix_options) if query_options.nil?
"#{prefix(prefix_options)}#{collection_name}/#{URI.parser.escape id.to_s}#{format_extension}#{query_string(query_options)}"
"#{prefix(prefix_options)}#{collection_name}/#{URI.encode_www_form_component(id.to_s)}#{format_extension}#{query_string(query_options)}"
end

# Gets the element url for the given ID in +id+. If the +query_options+ parameter is omitted, Rails
Expand Down
2 changes: 1 addition & 1 deletion test/cases/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ def test_custom_element_path
assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, person_id: 1)
assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, "person_id" => 1)
assert_equal "/people/Greg/addresses/1.json", StreetAddress.element_path(1, "person_id" => "Greg")
assert_equal "/people/ann%20mary/addresses/ann%20mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
assert_equal "/people/ann%20mary/addresses/ann+mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
end

def test_custom_element_path_without_required_prefix_param
Expand Down
16 changes: 16 additions & 0 deletions test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,20 @@ def test_find_single_by_symbol_from
david = Person.find(:one, from: :leader)
assert_equal "David", david.name
end

def test_find_identifier_encoding
ActiveResource::HttpMock.respond_to { |m| m.get "/people/%3F.json", {}, @david }

david = Person.find("?")

assert_equal "David", david.name
end

def test_find_identifier_encoding_for_path_traversal
ActiveResource::HttpMock.respond_to { |m| m.get "/people/..%2F.json", {}, @david }

david = Person.find("../")

assert_equal "David", david.name
end
end

0 comments on commit 0de18f7

Please sign in to comment.