Limit url parameter-based redirects to local urls

uffd/__init__.py

@@ -17,6 +17,7 @@ from uffd.database import db, SQLAlchemyJSON
 import uffd.ldap
 from uffd.template_helper import register_template_helper
 from uffd.navbar import setup_navbar
+from uffd.secure_redirect import secure_local_redirect
 # pylint: enable=wrong-import-position
 
 def load_config_file(app, cfg_name, silent=False):
@@ -97,7 +98,7 @@ def create_app(test_config=None): # pylint: disable=too-many-locals
 
 	@app.route('/lang', methods=['POST'])
 	def setlang(): #pylint: disable=unused-variable
-		resp = redirect(request.values.get('ref', '/'))
+		resp = secure_local_redirect(request.values.get('ref', '/'))
 		if 'lang' in request.values:
 			resp.set_cookie('language', request.values['lang'])
 		return resp

uffd/invite/templates/invite/use.html

@@ -33,7 +33,7 @@
 				<form method="POST" action="{{ url_for("invite.grant", token=invite.token) }}" class="mb-2">
 					<button type="submit" class="btn btn-primary btn-block">Add the roles to your account now</button>
 				</form>
-				<a href="{{ url_for("session.logout", ref=url_for("session.login", ref=request.url)) }}" class="btn btn-secondary btn-block">Logout and switch to a different account</a>
+				<a href="{{ url_for("session.logout", ref=url_for("session.login", ref=request.full_path)) }}" class="btn btn-secondary btn-block">Logout and switch to a different account</a>
 			{% endif %}
 			{% if invite.allow_signup %}
 				<a href="{{ url_for("session.logout", ref=url_for("invite.signup_start", token=invite.token)) }}" class="btn btn-secondary btn-block">Logout to register a new account</a>
@@ -43,7 +43,7 @@
 				<a href="{{ url_for("invite.signup_start", token=invite.token) }}" class="btn btn-primary btn-block">Register a new account</a>
 			{% endif %}
 			{% if invite.roles %}
-				<a href="{{ url_for("session.login", ref=request.url) }}" class="btn btn-primary btn-block">Login and add the roles to your account</a>
+				<a href="{{ url_for("session.login", ref=request.full_path) }}" class="btn btn-primary btn-block">Login and add the roles to your account</a>
 			{% endif %}
 		{% endif %}
 	</div>

uffd/mfa/views.py

@@ -10,6 +10,7 @@ from uffd.mfa.models import MFAMethod, TOTPMethod, WebauthnMethod, RecoveryCodeM
 from uffd.session.views import login_required, login_required_pre_mfa, set_request_user
 from uffd.user.models import User
 from uffd.csrf import csrf_protect
+from uffd.secure_redirect import secure_local_redirect
 from uffd.ratelimit import Ratelimit, format_delay
 
 bp = Blueprint('mfa', __name__, template_folder='templates', url_prefix='/mfa/')
@@ -213,7 +214,7 @@ def auth():
 		session['user_mfa'] = True
 		set_request_user()
 	if session.get('user_mfa'):
-		return redirect(request.values.get('ref', url_for('index')))
+		return secure_local_redirect(request.values.get('ref', url_for('index')))
 	return render_template('mfa/auth.html', ref=request.values.get('ref'))
 
 @bp.route('/auth', methods=['POST'])
@@ -227,7 +228,7 @@ def auth_finish():
 		if method.verify(request.form['code']):
 			session['user_mfa'] = True
 			set_request_user()
-			return redirect(request.values.get('ref', url_for('index')))
+			return secure_local_redirect(request.values.get('ref', url_for('index')))
 	for method in request.user_pre_mfa.mfa_recovery_codes:
 		if method.verify(request.form['code']):
 			db.session.delete(method)
@@ -240,7 +241,7 @@ def auth_finish():
 			if len(request.user_pre_mfa.mfa_recovery_codes) <= 5:
 				flash(_('You only have a few recovery codes remaining. Make sure to generate new ones before they run out.'))
 				return redirect(url_for('mfa.setup'))
-			return redirect(request.values.get('ref', url_for('index')))
+			return secure_local_redirect(request.values.get('ref', url_for('index')))
 	mfa_ratelimit.log(request.user_pre_mfa.dn)
 	flash(_('Two-factor authentication failed'))
 	return redirect(url_for('mfa.auth', ref=request.values.get('ref')))

uffd/oauth2/views.py

@@ -8,6 +8,7 @@ from sqlalchemy.exc import IntegrityError
 
 from uffd.ratelimit import host_ratelimit, format_delay
 from uffd.database import db
+from uffd.secure_redirect import secure_local_redirect
 from uffd.session.models import DeviceLoginConfirmation
 from .models import OAuth2Client, OAuth2Grant, OAuth2Token, OAuth2DeviceLoginInitiation
 
@@ -91,7 +92,7 @@ def authorize(*args, **kwargs): # pylint: disable=unused-argument
 		host_delay = host_ratelimit.get_delay()
 		if host_delay:
 			flash('We received too many requests from your ip address/network! Please wait at least %s.'%format_delay(host_delay))
-			return redirect(url_for('session.login', ref=request.url, devicelogin=True))
+			return redirect(url_for('session.login', ref=request.full_path, devicelogin=True))
 		host_ratelimit.log()
 		initiation = OAuth2DeviceLoginInitiation(oauth2_client_id=client.client_id)
 		db.session.add(initiation)
@@ -102,7 +103,7 @@ def authorize(*args, **kwargs): # pylint: disable=unused-argument
 			return redirect(url_for('session.login', ref=request.values['ref'], devicelogin=True))
 		session['devicelogin_id'] = initiation.id
 		session['devicelogin_secret'] = initiation.secret
-		return redirect(url_for('session.devicelogin', ref=request.url))
+		return redirect(url_for('session.devicelogin', ref=request.full_path))
 	elif 'devicelogin_id' in session and 'devicelogin_secret' in session and 'devicelogin_confirmation' in session:
 		initiation = OAuth2DeviceLoginInitiation.query.filter_by(id=session['devicelogin_id'], secret=session['devicelogin_secret'],
 		                                                         oauth2_client_id=client.client_id).one_or_none()
@@ -112,12 +113,12 @@ def authorize(*args, **kwargs): # pylint: disable=unused-argument
 		del session['devicelogin_confirmation']
 		if not initiation or initiation.expired or not confirmation:
 			flash('Device login failed')
-			return redirect(url_for('session.login', ref=request.url, devicelogin=True))
+			return redirect(url_for('session.login', ref=request.full_path, devicelogin=True))
 		request.oauth2_user = confirmation.user
 		db.session.delete(initiation)
 		db.session.commit()
 	else:
-		return redirect(url_for('session.login', ref=request.url, devicelogin=True))
+		return redirect(url_for('session.login', ref=request.full_path, devicelogin=True))
 	# Here we would normally ask the user, if he wants to give the requesting
 	# service access to his data. Since we only have trusted services (the
 	# clients defined in the server config), we don't ask for consent.
@@ -163,7 +164,7 @@ def inject_logout_params(endpoint, values):
 @bp.route('/logout')
 def logout():
 	if not request.values.get('client_ids'):
-		return redirect(request.values.get('ref', '/'))
+		return secure_local_redirect(request.values.get('ref', '/'))
 	client_ids = request.values['client_ids'].split(',')
 	clients = [OAuth2Client.from_id(client_id) for client_id in client_ids]
 	return render_template('oauth2/logout.html', clients=clients)

uffd/secure_redirect.py

+from flask import redirect
+
+def secure_local_redirect(target):
+	# Reject URLs that include a scheme or host part
+	if not target.startswith('/') or target.startswith('//'):
+		target = '/'
+	return redirect(target)

uffd/session/views.py

@@ -7,6 +7,7 @@ from flask_babel import gettext as _
 
 from uffd.database import db
 from uffd.csrf import csrf_protect
+from uffd.secure_redirect import secure_local_redirect
 from uffd.user.models import User
 from uffd.ldap import ldap, test_user_bind, LDAPInvalidDnError, LDAPBindError, LDAPPasswordIsMandatoryError
 from uffd.ratelimit import Ratelimit, host_ratelimit, format_delay
@@ -111,7 +112,7 @@ def login_required_pre_mfa(no_redirect=False):
 				if no_redirect:
 					abort(403)
 				flash(_('You need to login first'))
-				return redirect(url_for('session.login', ref=request.url))
+				return redirect(url_for('session.login', ref=request.full_path))
 			return func(*args, **kwargs)
 		return decorator
 	return wrapper
@@ -122,9 +123,9 @@ def login_required(group=None):
 		def decorator(*args, **kwargs):
 			if not request.user_pre_mfa:
 				flash(_('You need to login first'))
-				return redirect(url_for('session.login', ref=request.url))
+				return redirect(url_for('session.login', ref=request.full_path))
 			if not request.user:
-				return redirect(url_for('mfa.auth', ref=request.url))
+				return redirect(url_for('mfa.auth', ref=request.full_path))
 			if not request.user.is_in_group(group):
 				flash(_('Access denied'))
 				return redirect(url_for('index'))
@@ -135,7 +136,7 @@ def login_required(group=None):
 @bp.route("/login/device/start")
 def devicelogin_start():
 	session['devicelogin_started'] = True
-	return redirect(request.values['ref'])
+	return secure_local_redirect(request.values['ref'])
 
 @bp.route("/login/device")
 def devicelogin():
@@ -160,7 +161,7 @@ def devicelogin_submit():
 		flash('Invalid confirmation code')
 		return render_template('session/devicelogin.html', ref=request.values.get('ref'), initiation=initiation)
 	session['devicelogin_confirmation'] = confirmation.id
-	return redirect(request.values['ref'])
+	return secure_local_redirect(request.values['ref'])
 
 @bp.route("/device")
 @login_required()

uffd/templates/base.html

@@ -74,7 +74,7 @@
 					{% if config['LANGUAGES']|length > 1 %}
 					<li class="nav-item">
 						<form class="language-switch py-2 pr-1" method="POST" style="margin-left: -5px;" action="{{ url_for('setlang') }}">
-							<input type="hidden" name="ref" value="{{ request.url }}">
+							<input type="hidden" name="ref" value="{{ request.full_path }}">
 							<select name="lang" class="bg-dark" style="border: 0px; color: rgba(255, 255, 255, 0.5);" onchange="$('.language-switch').submit()">
 								{% for identifier, name in config['LANGUAGES'].items() %}
 									<option value="{{ identifier }}" {{ 'selected' if identifier == get_locale() }}>{{ name }}</option>