diff --git a/uffd/__init__.py b/uffd/__init__.py index 989ddab8fbcc52155b6a223bb35674c514f13fc0..293b47a1eccb8f929657e8e2a886a792bb7ad9f5 100644 --- a/uffd/__init__.py +++ b/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 diff --git a/uffd/invite/templates/invite/use.html b/uffd/invite/templates/invite/use.html index 4d5b53adb56d9afda80647ddbd25dc13c2c00f4e..c852f3db7e0c2f41ce111a7d0c27150b231cfc3b 100644 --- a/uffd/invite/templates/invite/use.html +++ b/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> diff --git a/uffd/mfa/views.py b/uffd/mfa/views.py index 133df9689d5e2436487d7e212907c11d25743c27..287b4ab03d34d74e905a297428461f03a79490f3 100644 --- a/uffd/mfa/views.py +++ b/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'))) diff --git a/uffd/oauth2/views.py b/uffd/oauth2/views.py index 30957deb14556cc99ee87977ba784515c8ebaf27..ed39aecddf9688e29f76a930780ed1f2af82149b 100644 --- a/uffd/oauth2/views.py +++ b/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) diff --git a/uffd/secure_redirect.py b/uffd/secure_redirect.py new file mode 100644 index 0000000000000000000000000000000000000000..69fa67cc40979c7447f6f31cc6abb07a67885a5c --- /dev/null +++ b/uffd/secure_redirect.py @@ -0,0 +1,7 @@ +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) diff --git a/uffd/session/views.py b/uffd/session/views.py index 8d095a2cd710c208fb26ac8658beb9130e6f6e33..b558b2745e5a1260383ea232bbbd94c5a41019ff 100644 --- a/uffd/session/views.py +++ b/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() diff --git a/uffd/templates/base.html b/uffd/templates/base.html index f07351bcdd8ee348f3e2d2598368175edc77076f..58ef8d722f13ae92f21911727da38d066914df65 100644 --- a/uffd/templates/base.html +++ b/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>