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

Overview Repositories Revisions Requests Users Attributes Meta

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

Index: Flask-Security-3.0.0/flask_security/core.py
===================================================================
--- Flask-Security-3.0.0.orig/flask_security/core.py
+++ Flask-Security-3.0.0/flask_security/core.py
@@ -12,6 +12,7 @@
 """
 
 from datetime import datetime
+import re
 
 import pkg_resources
 from flask import current_app, render_template
@@ -63,6 +64,8 @@ _default_config = {
     'POST_RESET_VIEW': None,
     'POST_CHANGE_VIEW': None,
     'UNAUTHORIZED_VIEW': lambda: 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',
@@ -339,6 +342,8 @@ def _get_state(app, datastore, anonymous
         _send_mail_task=None,
         _unauthorized_callback=None
     ))
+    if "redirect_validate_re" in kwargs:
+        kwargs["_redirect_validate_re"] = re.compile(kwargs["redirect_validate_re"])
 
     for key, value in _default_forms.items():
         if key not in kwargs or not kwargs[key]:
Index: Flask-Security-3.0.0/flask_security/utils.py
===================================================================
--- Flask-Security-3.0.0.orig/flask_security/utils.py
+++ Flask-Security-3.0.0/flask_security/utils.py
@@ -269,6 +269,34 @@ 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)
@@ -276,6 +304,9 @@ def validate_redirect_url(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-3.0.0/tests/test_misc.py
===================================================================
--- Flask-Security-3.0.0.orig/tests/test_misc.py
+++ Flask-Security-3.0.0/tests/test_misc.py
@@ -17,7 +17,8 @@ from flask_security.forms import ChangeP
     RegisterForm, ResetPasswordForm, SendConfirmationForm, StringField, \
     email_required, email_validator, valid_user_email
 from flask_security.utils import capture_reset_password_requests, \
-    encode_string, hash_data, string_types, verify_hash
+    encode_string, hash_data, string_types, verify_hash, \
+    validate_redirect_url
 
 
 @pytest.mark.recoverable()
@@ -280,3 +281,19 @@ def test_custom_forms_via_config(app, sq
 def test_without_babel(client):
     response = client.get('/login')
     assert b'Login' in response.data
+
+
+@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

Places