diff --git a/src/octoprint/server/util/__init__.py b/src/octoprint/server/util/__init__.py index 3137115d5e..157959d530 100644 --- a/src/octoprint/server/util/__init__.py +++ b/src/octoprint/server/util/__init__.py @@ -421,3 +421,23 @@ def require_login(*permissions): ) return None + + +def validate_local_redirect(url, allowed_paths): + """Validates the given local redirect URL against the given allowed paths. + + An `url` is valid for a local redirect if it has neither scheme nor netloc defined, + and its path is one of the given allowed paths. + + Args: + url (str): URL to validate + allowed_paths (List[str]): List of allowed paths, only paths contained + will be considered valid. + + Returns: + bool: Whether the `url` passed validation or not. + """ + from urllib.parse import urlparse + + parsed = urlparse(url) + return parsed.scheme == "" and parsed.netloc == "" and parsed.path in allowed_paths diff --git a/src/octoprint/server/views.py b/src/octoprint/server/views.py index d1db66b267..5757ce6c40 100644 --- a/src/octoprint/server/views.py +++ b/src/octoprint/server/views.py @@ -8,7 +8,6 @@ import os import re from collections import defaultdict -from urllib.parse import urlparse from flask import ( Response, @@ -39,7 +38,11 @@ preemptiveCache, userManager, ) -from octoprint.server.util import has_permissions, require_login_with +from octoprint.server.util import ( + has_permissions, + require_login_with, + validate_local_redirect, +) from octoprint.settings import settings from octoprint.util import sv, to_bytes, to_unicode from octoprint.util.version import get_python_version_string @@ -173,9 +176,9 @@ def login(): default_redirect_url = request.script_root + url_for("index") redirect_url = request.args.get("redirect", default_redirect_url) + allowed_paths = [url_for("index"), url_for("recovery")] - parsed = urlparse(redirect_url) # check if redirect url is valid - if parsed.scheme != "" or parsed.netloc != "": + if not validate_local_redirect(redirect_url, allowed_paths): _logger.warning( f"Got an invalid redirect URL with the login attempt, misconfiguration or attack attempt: {redirect_url}" ) diff --git a/tests/server/util/test_util.py b/tests/server/util/test_util.py new file mode 100644 index 0000000000..f455047eb4 --- /dev/null +++ b/tests/server/util/test_util.py @@ -0,0 +1,36 @@ +__license__ = "GNU Affero General Public License http://www.gnu.org/licenses/agpl.html" +__copyright__ = "Copyright (C) 2022 The OctoPrint Project - Released under terms of the AGPLv3 License" + +import pytest + + +@pytest.mark.parametrize( + "url,paths,expected", + [ + # various default UI URLs + ("/", ["/", "/recovery/"], True), + ("/?", ["/", "/recovery/"], True), + ("/?l10n=de", ["/", "/recovery/"], True), + ("/?l10n=de&", ["/", "/recovery/"], True), + ("/octoprint/", ["/octoprint/", "/octoprint/recovery/"], True), + # various recovery URLs + ("/recovery/", ["/", "/recovery/"], True), + ("/recovery/?", ["/", "/recovery/"], True), + ("/recovery/?l10n=de", ["/", "/recovery/"], True), + ("/octoprint/recovery/?l10n=de", ["/octoprint/", "/octoprint/recovery/"], True), + # various external URLs + ("http://example.com", ["/", "/recovery/"], False), + ("https://example.com", ["/", "/recovery/"], False), + ("//example.com", ["/", "/recovery/"], False), + ("/\\/\\example.com", ["/", "/recovery/"], False), + (" /\\/\\example.com", ["/", "/recovery/"], False), + ("\\/\\/example.com", ["/", "/recovery/"], False), + (" \\/\\/example.com", ["/", "/recovery/"], False), + # other stuff + ("javascript:alert(document.cookie)", ["/", "/recovery/"], False), + ], +) +def test_validate_local_redirect(url, paths, expected): + from octoprint.server.util import validate_local_redirect + + assert validate_local_redirect(url, paths) == expected