chore: Enhance is_safe_redirect_url

CONTRIBUTING.rst

@@ -69,7 +69,7 @@ Using Postgres
 
 .. code-block:: bash
 
-    $ nosetests -v flask_appbuilder.tests.test_A_fixture
+    $ nosetests -v flask_appbuilder.tests.A_fixture
 
 4 - Run a single test
 

flask_appbuilder/tests/security/test_mvc_security.py

@@ -109,30 +109,100 @@ def test_db_login_valid_next_url(self):
         )
         assert response.location == "http://localhost/users/list/"
 
-    def test_db_login_valid_full_next_url(self):
+    def test_db_login_valid_http_scheme_url(self):
         """
-        Test Security valid full next URL
+        Test Security valid http scheme next URL
         """
         self.browser_logout(self.client)
         response = self.browser_login(
             self.client,
             USERNAME_ADMIN,
             PASSWORD_ADMIN,
-            next_url="http://localhost/users/add",
+            next_url="http://localhost/path",
             follow_redirects=False,
         )
-        assert response.location == "http://localhost/users/add"
+        assert response.location == "http://localhost/path"
 
-    def test_db_login_invalid_next_url(self):
+    def test_db_login_valid_https_scheme_url(self):
         """
-        Test Security invalid next URL
+        Test Security valid https scheme next URL
         """
         self.browser_logout(self.client)
         response = self.browser_login(
             self.client,
             USERNAME_ADMIN,
             PASSWORD_ADMIN,
-            next_url="https://www.google.com",
+            next_url="https://localhost/path",
+            follow_redirects=False,
+        )
+        assert response.location == "https://localhost/path"
+
+    def test_db_login_invalid_external_next_url(self):
+        """
+        Test Security invalid external next URL
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            PASSWORD_ADMIN,
+            next_url="https://google.com",
+            follow_redirects=False,
+        )
+        assert response.location == "http://localhost/"
+
+    def test_db_login_invalid_scheme_next_url(self):
+        """
+        Test Security invalid scheme next URL
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            PASSWORD_ADMIN,
+            next_url="ftp://sample",
+            follow_redirects=False,
+        )
+        assert response.location == "http://localhost/"
+
+    def test_db_login_invalid_localhost_file_next_url(self):
+        """
+        Test Security invalid path to localhost file next URL
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            PASSWORD_ADMIN,
+            next_url="file:///path",
+            follow_redirects=False,
+        )
+        assert response.location == "http://localhost/"
+
+    def test_db_login_invalid_no_netloc_with_scheme_next_url(self):
+        """
+        Test Security invalid next URL with no netloc but with scheme
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            PASSWORD_ADMIN,
+            next_url="http:///sample.com ",
+            follow_redirects=False,
+        )
+        assert response.location == "http://localhost/"
+
+    def test_db_login_invalid_control_characters_next_url(self):
+        """
+        Test Security invalid next URL with control characters
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            PASSWORD_ADMIN,
+            next_url=u"\u0001" + "sample.com",
             follow_redirects=False,
         )
         assert response.location == "http://localhost/"

flask_appbuilder/tests/test_mvc_oauth.py

@@ -86,7 +86,7 @@ def test_oauth_login_next_check(self):
 
         self.appbuilder.sm.oauth_remotes = {"google": OAuthRemoteMock()}
 
-        raw_state = {"next": ["http://www.google.com"]}
+        raw_state = {"next": ["ftp://sample"]}
         state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
 
         response = client.get(f"/oauth-authorized/google?state={state}")

flask_appbuilder/utils/base.py

@@ -1,6 +1,7 @@
 import logging
 from typing import Any, Callable
-from urllib.parse import urljoin, urlparse
+import unicodedata
+from urllib.parse import urlparse
 
 from flask import current_app, request
 from flask_babel import gettext
@@ -11,11 +12,24 @@
 
 
 def is_safe_redirect_url(url: str) -> bool:
+    if url.startswith("///"):
+        return False
+    try:
+        url_info = urlparse(url)
+    except ValueError:
+        return False
+    if not url_info.netloc and url_info.scheme:
+        return False
+    if unicodedata.category(url[0])[0] == "C":
+        return False
+    scheme = url_info.scheme
+    # Consider URLs without a scheme (e.g. //example.com/p) to be http.
+    if not url_info.scheme and url_info.netloc:
+        scheme = "http"
+    valid_schemes = ["http", "https"]
     host_url = urlparse(request.host_url)
-    redirect_url = urlparse(urljoin(request.host_url, url))
-    return (
-        redirect_url.scheme in ("http", "https")
-        and host_url.netloc == redirect_url.netloc
+    return (not url_info.netloc or url_info.netloc == host_url.netloc) and (
+        not scheme or scheme in valid_schemes
     )