SECURITY: The SSO return_path was an open redirect

This security fix needs SSO to be configured, and the user has to go
through the entire auth process before being redirected to the wrong host so
it is probably lower priority for most installs.
This commit is contained in:
Robin Ward 2015-01-22 12:20:17 -05:00
parent e948dc88d1
commit b3a2c0c45b
2 changed files with 45 additions and 0 deletions

View File

@ -72,6 +72,17 @@ class SessionController < ApplicationController
else
log_on_user user
end
# If it's not a relative URL check the host
if return_path !~ /^\/[^\/]/
begin
uri = URI(return_path)
return_path = "/" unless uri.host == Discourse.current_hostname
rescue
return_path = "/"
end
end
redirect_to return_path
else
render text: I18n.t("sso.not_found"), status: 500

View File

@ -26,6 +26,8 @@ describe SessionController do
@sso_url = "http://somesite.com/discourse_sso"
@sso_secret = "shjkfdhsfkjh"
request.host = Discourse.current_hostname
SiteSetting.enable_sso = true
SiteSetting.sso_url = @sso_url
SiteSetting.sso_secret = @sso_secret
@ -79,7 +81,39 @@ describe SessionController do
logged_on_user = Discourse.current_user_provider.new(request.env).current_user
expect(logged_on_user.admin).to eq(true)
end
it 'redirects to a non-relative url' do
sso = get_sso("#{Discourse.base_url}/b/")
sso.external_id = '666' # the number of the beast
sso.email = 'bob@bob.com'
sso.name = 'Sam Saffron'
sso.username = 'sam'
get :sso_login, Rack::Utils.parse_query(sso.payload)
expect(response).to redirect_to('/b/')
end
it 'redirects to root if the host of the return_path is different' do
sso = get_sso('//eviltrout.com')
sso.external_id = '666' # the number of the beast
sso.email = 'bob@bob.com'
sso.name = 'Sam Saffron'
sso.username = 'sam'
get :sso_login, Rack::Utils.parse_query(sso.payload)
expect(response).to redirect_to('/')
end
it 'redirects to root if the host of the return_path is different' do
sso = get_sso('http://eviltrout.com')
sso.external_id = '666' # the number of the beast
sso.email = 'bob@bob.com'
sso.name = 'Sam Saffron'
sso.username = 'sam'
get :sso_login, Rack::Utils.parse_query(sso.payload)
expect(response).to redirect_to('/')
end
it 'allows you to create an account' do