Skip to content

Commit 0de18f7

Browse files
committed
Properly encode ID parameters to avoid possible information leak
[CVE-2020-8151]
1 parent 326b452 commit 0de18f7

File tree

3 files changed

+18
-2
lines changed

3 files changed

+18
-2
lines changed

lib/active_resource/base.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ def element_path(id, prefix_options = {}, query_options = nil)
772772
check_prefix_options(prefix_options)
773773

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

778778
# Gets the element url for the given ID in +id+. If the +query_options+ parameter is omitted, Rails

test/cases/base_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ def test_custom_element_path
688688
assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, person_id: 1)
689689
assert_equal "/people/1/addresses/1.json", StreetAddress.element_path(1, "person_id" => 1)
690690
assert_equal "/people/Greg/addresses/1.json", StreetAddress.element_path(1, "person_id" => "Greg")
691-
assert_equal "/people/ann%20mary/addresses/ann%20mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
691+
assert_equal "/people/ann%20mary/addresses/ann+mary.json", StreetAddress.element_path(:'ann mary', "person_id" => "ann mary")
692692
end
693693

694694
def test_custom_element_path_without_required_prefix_param

test/cases/finder_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,4 +172,20 @@ def test_find_single_by_symbol_from
172172
david = Person.find(:one, from: :leader)
173173
assert_equal "David", david.name
174174
end
175+
176+
def test_find_identifier_encoding
177+
ActiveResource::HttpMock.respond_to { |m| m.get "/people/%3F.json", {}, @david }
178+
179+
david = Person.find("?")
180+
181+
assert_equal "David", david.name
182+
end
183+
184+
def test_find_identifier_encoding_for_path_traversal
185+
ActiveResource::HttpMock.respond_to { |m| m.get "/people/..%2F.json", {}, @david }
186+
187+
david = Person.find("../")
188+
189+
assert_equal "David", david.name
190+
end
175191
end

0 commit comments

Comments
 (0)