Fix possible open-redirect with ALLOW_SUBDOMAIN option. (#1223)

As with the prior path-based open-redirect - the solution is to quote the hostname portion of the netloc.

Note that because we can get a 'next' location either via a query param (where Werkzeug unescapes it) OR as part of the form
where it ISN'T escaped - it is possible the resultant redirect URL will be double escaped.

closes #1222

Author: jwag956

CHANGES.rst

@@ -3,6 +3,15 @@ Flask-Security Changelog
 
 Here you can see the full list of changes between each Flask-Security release.
 
+Version 5.8.1
+-------------
+
+Released TBD
+
+Fixes
++++++
+- (:issue:`1222`) Possible open-redirect with ALLOW_SUBDOMAIN option.
+
 Version 5.8.0
 -------------
 

flask_security/utils.py

@@ -763,14 +763,29 @@ def get_post_action_redirect(
     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
+    By the time we get the URL it may or may not have been unencoded - if part of a
+    query string, then it has been, but if set in the form, not.
+    This means we can't really determine
     if it is 'valid' since it appears that '/'s can appear in the URL if escaped.
 
     The solution is to simply 'quote' the path.
+
+    netloc has a same issue: https://amazon.com\\.lp.com will cause many
+    browsers to redirect to amazon.com
     """
     rurl = propagate_next(find_redirect(config_key), next_loc)
-    scheme, netloc, path, query, fragment = urlsplit(rurl)
-    safe_url = urlunsplit((scheme, netloc, quote(path), query, fragment))
+
+    u = urlsplit(rurl)
+    userinfo = ""
+    if u.username or u.password:
+        userinfo = f"{u.username}:{u.password}@"
+    hostname = quote(u.hostname) if u.hostname else ""
+    if u.port:
+        netloc = f"{userinfo}{hostname}:{u.port}"
+    else:
+        netloc = f"{userinfo}{hostname}"
+
+    safe_url = urlunsplit((u.scheme, netloc, quote(u.path), u.query, u.fragment))
     return safe_url
 
 

tests/test_misc.py

@@ -71,6 +71,7 @@
     uia_email_mapper,
     uia_phone_mapper,
     verify_hash,
+    get_post_action_redirect,
 )
 from flask_security.core import _get_serializer
 
@@ -1367,18 +1368,73 @@ def test_open_redirect(app, client, get_message):
         ("//github.com", ""),
         ("\t//github.com", "%09//github.com"),
     ]
-    for i, o in test_urls:
-        data = dict(email="matt@lp.com", password="password", next=i)
-        response = client.post("/login", data=data, follow_redirects=False)
-        if response.status_code in [302, 303]:
-            # this means it passed form validation but should have been quoted
-            assert check_location(app, response.location, o)
-        elif response.status_code == 200:
-            # should have failed form validation
-            assert get_message("INVALID_REDIRECT") in response.data
-        else:
-            raise AssertionError("Bad response code")
-        logout(client)
+    for nextloc in ["form", "query"]:
+        for i, o in test_urls:
+            if nextloc == "form":
+                data = dict(email="matt@lp.com", password="password", next=i)
+                response = client.post("/login", data=data, follow_redirects=False)
+            else:
+                data = dict(email="matt@lp.com", password="password")
+                response = client.post(
+                    f"/login?next={i}", data=data, follow_redirects=False
+                )
+            if response.status_code in [302, 303]:
+                # this means it passed form validation but should have been quoted
+                assert check_location(app, response.location, o)
+            elif response.status_code == 200:
+                # should have failed form validation
+                assert get_message("INVALID_REDIRECT") in response.data
+            else:
+                raise AssertionError("Bad response code")
+            logout(client)
+
+
+@pytest.mark.settings(redirect_allow_subdomains=True)
+def test_open_redirect_subdomain(app, client, get_message):
+    # As above - netloc is also susceptible to crazy escaping
+    # The first URL below, if cut-n-pasted into Chrome will end up at amazon.com
+    # The difference in response between in-form and query string is that
+    # the query string is unescaped by Werkzeug but the form value isn't.
+    app.config["SERVER_NAME"] = "lp.com"
+    test_urls = [
+        ("https://amazon.com\\.lp.com", "https://amazon.com%5C.lp.com", None),
+        (
+            "https://amazon.com%5C.lp.com",
+            "https://amazon.com%255C.lp.com",
+            "https://amazon.com%5C.lp.com",
+        ),
+        (
+            "https://jwag:mypass@amazon.com\\.lp.com",
+            "https://jwag:mypass@amazon.com%5C.lp.com",
+            None,
+        ),
+    ]
+    for nextloc in ["form", "query"]:
+        for i, o, oq in test_urls:
+            if nextloc == "form":
+                data = dict(email="matt@lp.com", password="password", next=i)
+                response = client.post("/login", data=data, follow_redirects=False)
+            else:
+                data = dict(email="matt@lp.com", password="password")
+                response = client.post(
+                    f"/login?next={i}", data=data, follow_redirects=False
+                )
+            if response.status_code in [302, 303]:
+                # this means it passed form validation but should have been quoted
+                assert check_location(
+                    app, response.location, oq if oq and (nextloc == "query") else o
+                )
+            logout(client)
+
+
+def test_get_post_action_redirect(app, client):
+    # test parts of get_post_action_redirect that are hard to get to via the client
+    # e.g. port
+    with app.test_request_context(base_url="https://lp.com:8080/"):
+        r = get_post_action_redirect(
+            "SECURITY_POST_LOGIN_VIEW", dict(next="https://lp.com:8080/myredirect")
+        )
+        assert r == "https://lp.com:8080/myredirect"
 
 
 def test_kwargs():

tests/view_scaffold.py

@@ -161,6 +161,7 @@ class WebAuthn(Model, sqla.FsWebAuthnMixin):
 def create_app() -> Flask:
     # Use real templates - not test templates...
     app = Flask("view_scaffold", template_folder="../")
+    app.config["SERVER_NAME"] = "localhost"
     app.config["DEBUG"] = True
     # SECRET_KEY generated using: secrets.token_urlsafe()
     app.config["SECRET_KEY"] = "pf9Wkove4IKEAXvy-cQkeDPhv9Cb3Ag-wyJILbq_dFw"