Skip to content

Commit ddfbc8d

Browse files
authored
Merge pull request #830 from jnunemaker/fix-ssrf-base-uri-bypass
fix: prevent SSRF via absolute URL bypassing base_uri
2 parents 05f38fd + 0529bcd commit ddfbc8d

File tree

4 files changed

+95
-1
lines changed

4 files changed

+95
-1
lines changed

lib/httparty/exceptions.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,8 @@ class DuplicateLocationHeader < ResponseError; end
5959

6060
# Exception that is raised when common network errors occur.
6161
class NetworkError < Foul; end
62+
63+
# Exception that is raised when an absolute URI is used that doesn't match
64+
# the configured base_uri, which could indicate an SSRF attempt.
65+
class UnsafeURIError < Foul; end
6266
end

lib/httparty/request.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ def uri
113113
new_uri = path.clone
114114
end
115115

116+
validate_uri_safety!(new_uri) unless redirect
117+
116118
# avoid double query string on redirects [#12]
117119
unless redirect
118120
new_uri.query = query_string(new_uri)
@@ -442,5 +444,23 @@ def encode_text(text, content_type)
442444
assume_utf16_is_big_endian: assume_utf16_is_big_endian
443445
).call
444446
end
447+
448+
def validate_uri_safety!(new_uri)
449+
return if options[:skip_uri_validation]
450+
451+
configured_base_uri = options[:base_uri]
452+
return unless configured_base_uri
453+
454+
normalized_base = options[:uri_adapter].parse(
455+
HTTParty.normalize_base_uri(configured_base_uri)
456+
)
457+
458+
return if new_uri.host == normalized_base.host
459+
460+
raise UnsafeURIError,
461+
"Requested URI '#{new_uri}' has host '#{new_uri.host}' but the " \
462+
"configured base_uri '#{normalized_base}' has host '#{normalized_base.host}'. " \
463+
"This request could send credentials to an unintended server."
464+
end
445465
end
446466
end

spec/httparty/request_spec.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,74 @@
384384
end
385385
end
386386
end
387+
388+
context "URI safety validation" do
389+
context "when base_uri is configured" do
390+
it "raises UnsafeURIError when path is an absolute URL with different host" do
391+
request = HTTParty::Request.new(
392+
Net::HTTP::Get,
393+
'http://evil.com/steal-data',
394+
base_uri: 'http://trusted.com'
395+
)
396+
expect { request.uri }.to raise_error(
397+
HTTParty::UnsafeURIError,
398+
/has host 'evil.com' but the configured base_uri .* has host 'trusted.com'/
399+
)
400+
end
401+
402+
it "allows requests when path host matches base_uri host" do
403+
request = HTTParty::Request.new(
404+
Net::HTTP::Get,
405+
'http://trusted.com/api/data',
406+
base_uri: 'http://trusted.com'
407+
)
408+
expect { request.uri }.not_to raise_error
409+
expect(request.uri.host).to eq('trusted.com')
410+
end
411+
412+
it "allows relative paths" do
413+
request = HTTParty::Request.new(
414+
Net::HTTP::Get,
415+
'/api/data',
416+
base_uri: 'http://trusted.com'
417+
)
418+
expect { request.uri }.not_to raise_error
419+
expect(request.uri.to_s).to eq('http://trusted.com/api/data')
420+
end
421+
422+
it "raises UnsafeURIError for network-relative URLs with different host" do
423+
@request.last_uri = URI.parse("https://trusted.com")
424+
@request.path = URI.parse("//evil.com/steal")
425+
@request.redirect = true
426+
@request.options[:base_uri] = 'http://trusted.com'
427+
428+
# During redirects, URI safety is not checked to allow legitimate redirects
429+
expect { @request.uri }.not_to raise_error
430+
end
431+
432+
it "can be bypassed with skip_uri_validation option" do
433+
request = HTTParty::Request.new(
434+
Net::HTTP::Get,
435+
'http://other.com/api/data',
436+
base_uri: 'http://trusted.com',
437+
skip_uri_validation: true
438+
)
439+
expect { request.uri }.not_to raise_error
440+
expect(request.uri.host).to eq('other.com')
441+
end
442+
end
443+
444+
context "when base_uri is not configured" do
445+
it "allows absolute URLs" do
446+
request = HTTParty::Request.new(
447+
Net::HTTP::Get,
448+
'http://any-server.com/api/data'
449+
)
450+
expect { request.uri }.not_to raise_error
451+
expect(request.uri.host).to eq('any-server.com')
452+
end
453+
end
454+
end
387455
end
388456

389457
describe "#setup_raw_request" do

spec/support/stub_response.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ def response.read_body(&block)
2929

3030
def stub_response(body, code = '200')
3131
code = code.to_s
32-
@request.options[:base_uri] ||= 'http://localhost'
32+
# Only set default base_uri if path is relative (has no host)
33+
# This avoids triggering URI safety validation for absolute URLs in tests
34+
@request.options[:base_uri] ||= 'http://localhost' if @request.path.relative?
3335
unless defined?(@http) && @http
3436
@http = Net::HTTP.new('localhost', 80)
3537
allow(@request).to receive(:http).and_return(@http)

0 commit comments

Comments
 (0)