Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:15-SP2
python-Flask-Security-Too
fix-open-redirect.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File fix-open-redirect.patch of Package python-Flask-Security-Too
Index: Flask-Security-Too-3.4.2/flask_security/core.py =================================================================== --- Flask-Security-Too-3.4.2.orig/flask_security/core.py +++ Flask-Security-Too-3.4.2/flask_security/core.py @@ -15,6 +15,7 @@ from datetime import datetime, timedelta import warnings import sys +import re import pkg_resources from flask import _request_ctx_stack, current_app, render_template @@ -161,6 +162,8 @@ _default_config = { "LOGIN_ERROR_VIEW": None, "REDIRECT_HOST": None, "REDIRECT_BEHAVIOR": None, + "REDIRECT_VALIDATE_MODE": None, + "REDIRECT_VALIDATE_RE": r"^/{4,}|\\{3,}|[\s\000-\037][/\\]{2,}", "FORGOT_PASSWORD_TEMPLATE": "security/forgot_password.html", "LOGIN_USER_TEMPLATE": "security/login_user.html", "REGISTER_USER_TEMPLATE": "security/register_user.html", @@ -623,6 +626,9 @@ def _get_state(app, datastore, anonymous ) ) + if "redirect_validate_re" in kwargs: + kwargs["_redirect_validate_re"] = re.compile(kwargs["redirect_validate_re"]) + if "login_manager" not in kwargs: kwargs["login_manager"] = _get_login_manager(app, anonymous_user) Index: Flask-Security-Too-3.4.2/flask_security/utils.py =================================================================== --- Flask-Security-Too-3.4.2.orig/flask_security/utils.py +++ Flask-Security-Too-3.4.2/flask_security/utils.py @@ -465,12 +465,43 @@ def url_for_security(endpoint, **values) def validate_redirect_url(url): + """Validate that the URL for redirect is relative. + Allowing an absolute redirect is a security issue - a so-called open-redirect. + Note that by default Werkzeug will always take this URL and make it relative + when setting the Location header - but that behavior can be overridden. + + The complexity here is that urlsplit() does pretty well, but browsers even today + May 2021 are very lenient in what they accept as URLs - for example: + next=\\\\github.com + next=%5C%5C%5Cgithub.com + next=/////github.com + next=%20\\\\github.com + next=%20///github.com + next=%20//github.com + next=%19////github.com - i.e. browser will strip control chars + next=%E2%80%8A///github.com - doesn't redirect! That is a unicode thin space. + + All will result in a null netloc and scheme from urlsplit - however many browsers + will gladly strip off uninteresting characters and convert backslashes to forward + slashes - and the cases above will actually cause a redirect to github.com + Sigh. + + Some articles claim that a relative url has to start with a '/' - but that isn't + strictly true. From: https://datatracker.ietf.org/doc/html/rfc3986#section-5 + a relative path can start with a "//", "/", a non-colon, or be empty. So it seems + that all the above URLs are valid. + By the time we get the URL, it has been unencoded - so we can't really determine + if it is 'valid' since it appears that '/'s can appear in the URL if escaped. + """ if url is None or url.strip() == "": return False url_next = urlsplit(url) url_base = urlsplit(request.host_url) if (url_next.netloc or url_next.scheme) and url_next.netloc != url_base.netloc: return False + if config_value("REDIRECT_VALIDATE_MODE") == "regex": + matcher = _security._redirect_validate_re.match(url) + return matcher is None return True Index: Flask-Security-Too-3.4.2/tests/test_misc.py =================================================================== --- Flask-Security-Too-3.4.2.orig/tests/test_misc.py +++ Flask-Security-Too-3.4.2/tests/test_misc.py @@ -56,6 +56,7 @@ from flask_security.utils import ( string_types, uia_phone_mapper, verify_hash, + validate_redirect_url, ) @@ -911,3 +912,19 @@ def test_verify_fresh_json(app, client, response = client.get("/fresh", headers=headers) assert response.status_code == 200 assert response.json["title"] == "Fresh Only" + + +@pytest.mark.settings(redirect_validate_mode="regex") +def test_validate_redirect(app, sqlalchemy_datastore): + """ + Test various possible URLs that urlsplit() shows as relative but + many browsers will interpret as absolute - and this have a + open-redirect vulnerability. Note this vulnerability only + is viable if the application sets autocorrect_location_header = False + """ + init_app_with_options(app, sqlalchemy_datastore) + with app.test_request_context("http://localhost:5001/login"): + assert not validate_redirect_url("\\\\\\github.com") + assert not validate_redirect_url(" //github.com") + assert not validate_redirect_url("\t//github.com") + assert not validate_redirect_url("//github.com") # this is normal urlsplit
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor