File fix-open-redirect.patch of Package python-Flask-Security-Too.26162
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