Skip to content

Commit 032f2c9

Browse files
committed
Open redirect vulnerability (#19577).
Patch by Holger Just. git-svn-id: http://svn.redmine.org/redmine/trunk@14560 e93f8b46-1217-0410-a6f0-8f06a7374b81
1 parent 6583c17 commit 032f2c9

File tree

2 files changed

+36
-9
lines changed

2 files changed

+36
-9
lines changed

Diff for: app/controllers/application_controller.rb

+26-8
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,8 @@ def back_url
396396

397397
def redirect_back_or_default(default, options={})
398398
back_url = params[:back_url].to_s
399-
if back_url.present? && valid_back_url?(back_url)
400-
redirect_to(back_url)
399+
if back_url.present? && valid_url = validate_back_url(back_url)
400+
redirect_to(valid_url)
401401
return
402402
elsif options[:referer]
403403
redirect_to_referer_or default
@@ -407,8 +407,9 @@ def redirect_back_or_default(default, options={})
407407
false
408408
end
409409

410-
# Returns true if back_url is a valid url for redirection, otherwise false
411-
def valid_back_url?(back_url)
410+
# Returns a validated URL string if back_url is a valid url for redirection,
411+
# otherwise false
412+
def validate_back_url(back_url)
412413
if CGI.unescape(back_url).include?('..')
413414
return false
414415
end
@@ -419,19 +420,36 @@ def valid_back_url?(back_url)
419420
return false
420421
end
421422

422-
if uri.host.present? && uri.host != request.host
423+
[:scheme, :host, :port].each do |component|
424+
if uri.send(component).present? && uri.send(component) != request.send(component)
425+
return false
426+
end
427+
uri.send(:"#{component}=", nil)
428+
end
429+
# Always ignore basic user:password in the URL
430+
uri.userinfo = nil
431+
432+
path = uri.to_s
433+
# Ensure that the remaining URL starts with a slash, followed by a
434+
# non-slash character or the end
435+
if path !~ %r{\A/([^/]|\z)}
423436
return false
424437
end
425438

426-
if uri.path.match(%r{/(login|account/register)})
439+
if path.match(%r{/(login|account/register)})
427440
return false
428441
end
429442

430-
if relative_url_root.present? && !uri.path.starts_with?(relative_url_root)
443+
if relative_url_root.present? && !path.starts_with?(relative_url_root)
431444
return false
432445
end
433446

434-
return true
447+
return path
448+
end
449+
private :validate_back_url
450+
451+
def valid_back_url?(back_url)
452+
!!validate_back_url(back_url)
435453
end
436454
private :valid_back_url?
437455

Diff for: test/functional/account_controller_test.rb

+10-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ def test_login_should_redirect_to_back_url_param
6363
# request.uri is "test.host" in test environment
6464
back_urls = [
6565
'http://test.host/issues/show/1',
66+
'http://test.host/',
6667
'/'
6768
]
6869
back_urls.each do |back_url|
@@ -108,7 +109,15 @@ def test_login_with_suburi_should_not_redirect_to_another_suburi
108109
'http://test.host/fake/issues',
109110
'http://test.host/redmine/../fake',
110111
'http://test.host/redmine/../fake/issues',
111-
'http://test.host/redmine/%2e%2e/fake'
112+
'http://test.host/redmine/%2e%2e/fake',
113+
'//test.foo/fake',
114+
'http://test.host//fake',
115+
'http://test.host/\n//fake',
116+
'//bar@test.foo',
117+
'//test.foo',
118+
'////test.foo',
119+
'@test.foo',
120+
'fake@test.foo'
112121
]
113122
back_urls.each do |back_url|
114123
post :login, :username => 'jsmith', :password => 'jsmith', :back_url => back_url

0 commit comments

Comments
 (0)