Bug#1031279: bullseye-pu: package flask-security/4.0.0-1+deb11u1
Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: flask-security@packages.debian.org
Control: affects -1 + src:flask-security
[ Reason ]
The version of flask-security in bullseye is affected by CVE-2021-23385.
https://security-tracker.debian.org/tracker/CVE-2021-23385
[ Impact ]
Without that fix users of Flask based application which using
get_post_logout_redirect and get_post_login_redirect functions might get
an bypassed URL validation and redirect a user to an arbitrary URL.
[ Tests ]
Upstream has added a test to check the code for catching any bypass
while adding the needed source code changes.
https://github.com/Flask-Middleware/flask-security/commit/e39bb04615050448c1b8ba4caa7dacc0edd3e405#diff-56f87108fb8c4605e56b4702938ff2211dd019c94ac130bfbc8016e6a9143dd0
I did not check this fix manually but run several test run while working
on updating flask-security in unstable at time.
[ Risks ]
The added debdiff is quite long as also a lot of documentation in
configuration.rst is added.
The relevant part of the source code changes to fix the CVE are rather
small and not very complex.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in (old)stable
[x] the issue is verified as fixed in unstable
[ Changes ]
Upstream extended the function validate_redirect_url() in a way if a
configuration value of REDIRECT_VALIDATE_MODE is matching 'regex' the
given URL will get validated.
Upstream also added some more documentation in general to the
documentation and also to functions that are used.
And some test is added to the internal test suite.
[ Other info ]
This fix is backported from
https://github.com/Flask-Middleware/flask-security/pull/489
and is included within the current version in unstable/testing.
Note: The version in bullseye is originally based on the upstream source
https://github.com/mattupstate/flask-security!
But this git tree isn't actively maintained any more and the active
development is happen now on https://github.com/Flask-Middleware/flask-security
where the version in unstable/testing is based on.
I was in conversation with the security team if this fix is worth to get
an update through stable-security. As the issue is tagged as
low-priority the suggestion was to fix the problem through a point
update for bullseye.
diff -Nru flask-security-4.0.0/debian/changelog flask-security-4.0.0/debian/changelog
--- flask-security-4.0.0/debian/changelog 2021-02-01 14:42:21.000000000 +0000
+++ flask-security-4.0.0/debian/changelog 2023-02-14 11:10:52.000000000 +0000
@@ -1,3 +1,12 @@
+flask-security (4.0.0-1+deb11u1) bullseye; urgency=medium
+
+ * Fix for CVE-2021-23385
+ Cherry pick partially PR #489 from the upstream project
+ (https://github.com/Flask-Middleware/flask-security/pull/489)
+ to fix Open Redirect Vulnerability aka CVE-2021-23385.
+
+ -- Carsten Schoenert <c.schoenert@t-online.de> Tue, 14 Feb 2023 11:10:52 +0000
+
flask-security (4.0.0-1) unstable; urgency=medium
* Team upload.
diff -Nru flask-security-4.0.0/debian/patches/0001-A-hopeful-fix-for-possible-open-redirect.patch flask-security-4.0.0/debian/patches/0001-A-hopeful-fix-for-possible-open-redirect.patch
--- flask-security-4.0.0/debian/patches/0001-A-hopeful-fix-for-possible-open-redirect.patch 1970-01-01 01:00:00.000000000 +0100
+++ flask-security-4.0.0/debian/patches/0001-A-hopeful-fix-for-possible-open-redirect.patch 2023-02-14 11:02:05.000000000 +0000
@@ -0,0 +1,298 @@
+From: jwag956 <jwag.wagner@gmail.com>
+Date: Sat, 29 May 2021 19:18:55 -0700
+Subject: A (hopeful) fix for possible open-redirect.
+
+While this is only an issue if the application sets the Werkzeug response variable:
+autocorrect_location_header = False - it none the less poses a small security concern.
+
+pyupgrade and black changed again .. sigh...
+pin read the docs sphinx versions.
+
+Closes: #486
+
+Forwared: https://github.com/Flask-Middleware/flask-security/pull/489
+---
+ docs/configuration.rst | 53 +++++++++++++++++++++++++++++++++++++++++++-
+ flask_security/core.py | 7 +++++-
+ flask_security/datastore.py | 2 +-
+ flask_security/decorators.py | 4 ++--
+ flask_security/utils.py | 31 ++++++++++++++++++++++++++
+ requirements/docs.txt | 6 ++---
+ tests/test_misc.py | 17 ++++++++++++++
+ tests/view_scaffold.py | 8 +++++++
+ 8 files changed, 120 insertions(+), 8 deletions(-)
+
+diff --git a/docs/configuration.rst b/docs/configuration.rst
+index 497bd1d..12144da 100644
+--- a/docs/configuration.rst
++++ b/docs/configuration.rst
+@@ -216,7 +216,7 @@ These configuration keys are used globally across all features.
+ .. py:data:: SECURITY_REDIRECT_ALLOW_SUBDOMAINS
+
+ If ``True`` then subdomains (and the root domain) of the top-level host set
+- by Flask's ``SERVER_NAME`` configuration will be allowed as post-login redirect targets.
++ by Flask's ``SERVER_NAME`` configuration will be allowed as post-view redirect targets.
+ This is beneficial if you wish to place your authentiation on one subdomain and
+ authenticated content on another, for example ``auth.domain.tld`` and ``app.domain.tld``.
+
+@@ -224,6 +224,51 @@ These configuration keys are used globally across all features.
+
+ .. versionadded:: 4.0.0
+
++.. py:data:: SECURITY_REDIRECT_VALIDATE_MODE
++
++ These 2 configuration options attempt to solve a open-redirect vulnerability
++ that can be exploited if an application sets the Werkzeug response option
++ ``autocorrect_location_header = False`` (it is ``True`` by default).
++ For numerous views (e.g. /login) Flask-Security allows callers to specify
++ a redirect upon successful completion (via the ?next parameter). So it is
++ possible for a user to be tricked into logging in to a legitimate site
++ and then redirected to a malicious site. Flask-Security attempts to
++ verify that redirects are always relative to overcome this security concern.
++ FS uses the standard Python library urlsplit() to parse the URL and verify
++ that the ``netloc`` hasn't been altered.
++ However, many browsers actually accept URLs that should be considered
++ relative and perform various stripping and conversion that can cause them
++ to be interpreted as absolute. A trivial example of this is:
++
++ .. line-block::
++ /login?next=%20///github.com
++
++ This will pass the urlsplit() test that it is relative - but many browsers
++ will simply strip off the space and interpret it as an absolute URL!
++ With the default configuration of Werkzeug this isn't an issue since it by
++ default modifies the Location Header to with the request ``netloc``. However
++ if the application sets the Werkzeug response option
++ ``autocorrect_location_header = False`` this will allow a redirect outside of
++ the application.
++
++ Setting this to ``"regex"`` will force the URL to be matched using the
++ pattern specified below. If a match occurs the URL is considered 'absolute'
++ and will be rejected.
++
++ Default: ``None``
++
++ .. versionadded:: 4.0.2
++
++.. py:data:: SECURITY_REDIRECT_VALIDATE_RE
++
++ This regex handles known patterns that can be exploited. Basically,
++ don't allow control characters or white-space followed by slashes (or
++ back slashes).
++
++ Default: ``r"^/{4,}|\\{3,}|[\s\000-\037][/\\]{2,}"``
++
++ .. versionadded:: 4.0.2
++
+ .. py:data:: SECURITY_CSRF_PROTECT_MECHANISMS
+
+ Authentication mechanisms that require CSRF protection.
+@@ -594,6 +639,8 @@ Login/Logout
+
+ Default: ``"/verify"``
+
++ .. versionadded:: 3.4.0
++
+
+ .. py:data:: SECURITY_VERIFY_TEMPLATE
+
+@@ -601,6 +648,8 @@ Login/Logout
+
+ Default: ``"security/verify.html"``.
+
++ .. versionadded:: 3.4.0
++
+ .. py:data:: SECURITY_POST_VERIFY_URL
+
+ Specifies the default view to redirect to after a user successfully re-authenticates either via
+@@ -610,6 +659,8 @@ Login/Logout
+
+ Default: ``None``.
+
++ .. versionadded:: 3.4.0
++
+ Registerable
+ ------------
+ .. py:data:: SECURITY_REGISTERABLE
+diff --git a/flask_security/core.py b/flask_security/core.py
+index ad4aab7..71b1482 100644
+--- a/flask_security/core.py
++++ b/flask_security/core.py
+@@ -7,11 +7,12 @@
+ :copyright: (c) 2012 by Matt Wright.
+ :copyright: (c) 2017 by CERN.
+ :copyright: (c) 2017 by ETH Zurich, Swiss Data Science Center.
+- :copyright: (c) 2019-2020 by J. Christopher Wagner (jwag).
++ :copyright: (c) 2019-2021 by J. Christopher Wagner (jwag).
+ :license: MIT, see LICENSE for more details.
+ """
+
+ from datetime import datetime, timedelta
++import re
+ import warnings
+
+ import pkg_resources
+@@ -156,6 +157,8 @@ _default_config = {
+ "REDIRECT_HOST": None,
+ "REDIRECT_BEHAVIOR": None,
+ "REDIRECT_ALLOW_SUBDOMAINS": False,
++ "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",
+@@ -581,6 +584,8 @@ def _get_state(app, datastore, anonymous_user=None, **kwargs):
+ _unauthz_handler=default_unauthz_handler,
+ )
+ )
++ 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)
+diff --git a/flask_security/datastore.py b/flask_security/datastore.py
+index 4693eac..773ec3a 100644
+--- a/flask_security/datastore.py
++++ b/flask_security/datastore.py
+@@ -319,7 +319,7 @@ class UserDatastore:
+ perms = ",".join(perms)
+ elif isinstance(perms, str):
+ # squash spaces.
+- perms = ",".join([p.strip() for p in perms.split(",")])
++ perms = ",".join(p.strip() for p in perms.split(","))
+ kwargs["permissions"] = perms
+
+ role = self.role_model(**kwargs)
+diff --git a/flask_security/decorators.py b/flask_security/decorators.py
+index a934af1..9c0e38e 100644
+--- a/flask_security/decorators.py
++++ b/flask_security/decorators.py
+@@ -504,7 +504,7 @@ def roles_accepted(*roles):
+ def wrapper(fn):
+ @wraps(fn)
+ def decorated_view(*args, **kwargs):
+- perm = Permission(*[RoleNeed(role) for role in roles])
++ perm = Permission(*(RoleNeed(role) for role in roles))
+ if perm.can():
+ return fn(*args, **kwargs)
+ if _security._unauthorized_callback:
+@@ -578,7 +578,7 @@ def permissions_accepted(*fsperms):
+ def wrapper(fn):
+ @wraps(fn)
+ def decorated_view(*args, **kwargs):
+- perm = Permission(*[FsPermNeed(fsperm) for fsperm in fsperms])
++ perm = Permission(*(FsPermNeed(fsperm) for fsperm in fsperms))
+ if perm.can():
+ return fn(*args, **kwargs)
+ if _security._unauthorized_callback:
+diff --git a/flask_security/utils.py b/flask_security/utils.py
+index 0dffcef..372db0b 100644
+--- a/flask_security/utils.py
++++ b/flask_security/utils.py
+@@ -498,6 +498,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)
+@@ -515,6 +543,9 @@ def validate_redirect_url(url):
+ return True
+ else:
+ return False
++ if config_value("REDIRECT_VALIDATE_MODE") == "regex":
++ matcher = _security._redirect_validate_re.match(url)
++ return matcher is None
+ return True
+
+
+diff --git a/requirements/docs.txt b/requirements/docs.txt
+index 0ace8b2..06c7d25 100644
+--- a/requirements/docs.txt
++++ b/requirements/docs.txt
+@@ -1,3 +1,3 @@
+-Pallets-Sphinx-Themes
+-Sphinx
+-sphinx-issues
++Pallets-Sphinx-Themes~=2.0
++Sphinx~=4.0
++sphinx-issues~=1.2
+diff --git a/tests/test_misc.py b/tests/test_misc.py
+index 3a5c3f3..a3e12bd 100644
+--- a/tests/test_misc.py
++++ b/tests/test_misc.py
+@@ -56,6 +56,7 @@ from flask_security.utils import (
+ hash_data,
+ send_mail,
+ uia_phone_mapper,
++ validate_redirect_url,
+ verify_hash,
+ )
+
+@@ -1011,3 +1012,19 @@ def test_post_security_with_application_root_and_views(app, sqlalchemy_datastore
+ response = client.get("/logout")
+ assert response.status_code == 302
+ assert response.headers["Location"] == "http://localhost/post_logout";
++
++
++@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
+diff --git a/tests/view_scaffold.py b/tests/view_scaffold.py
+index d522917..71a8237 100644
+--- a/tests/view_scaffold.py
++++ b/tests/view_scaffold.py
+@@ -159,6 +159,14 @@ def create_app():
+ add_user(user_datastore, test_acct, "password", ["admin"])
+ print("Created User: {} with password {}".format(test_acct, "password"))
+
++ @app.after_request
++ def allow_absolute_redirect(r):
++ # This is JUST to test odd possible redirects that look relative but are
++ # interpreted by browsers as absolute.
++ # DON'T SET THIS IN YOUR APPLICATION!
++ r.autocorrect_location_header = False
++ return r
++
+ @user_registered.connect_via(app)
+ def on_user_registered(myapp, user, confirm_token, **extra):
+ flash(f"To confirm {user.email} - go to /confirm/{confirm_token}")
diff -Nru flask-security-4.0.0/debian/patches/series flask-security-4.0.0/debian/patches/series
--- flask-security-4.0.0/debian/patches/series 1970-01-01 01:00:00.000000000 +0100
+++ flask-security-4.0.0/debian/patches/series 2023-02-14 11:02:05.000000000 +0000
@@ -0,0 +1 @@
+0001-A-hopeful-fix-for-possible-open-redirect.patch
Reply to: