File fix-open-redirect.patch of Package python-Flask-Security-Too.26162

Overview Repositories Revisions Requests Users Attributes Meta

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

Places

File fix-open-redirect.patch of Package python-Flask-Security-Too.26162

Places