diff --git a/src/fastapi_multiauth/oauth/utils.py b/src/fastapi_multiauth/oauth/utils.py index f3f12ad..5478dd0 100644 --- a/src/fastapi_multiauth/oauth/utils.py +++ b/src/fastapi_multiauth/oauth/utils.py @@ -98,9 +98,10 @@ def oauth_encode_state(url: str, state_token: str) -> str: def _destination_allowed(destination: str, allowed_hosts: Sequence[str]) -> bool: """Check a decoded destination against the open-redirect allowlist.""" - # Browsers normalize backslashes to slashes, turning "/\evil.com" into a - # scheme-relative redirect — reject them outright. - if "\\" in destination: + # Browsers normalize backslashes to slashes ("/\evil.com" becomes a + # scheme-relative redirect) and strip tabs/newlines before parsing + # ("/\t/evil.com" becomes "//evil.com") — reject control chars outright. + if "\\" in destination or any(ch <= " " for ch in destination): return False parsed = urlsplit(destination) if not parsed.scheme and not parsed.netloc: @@ -125,7 +126,8 @@ def oauth_decode_state( Args: state: Raw ``state`` query parameter from the callback. expected_state_token: Token stored before the authorization redirect; - a mismatch returns ``fallback``. + a mismatch — or an empty token (e.g. a session miss) — returns + ``fallback``. fallback: URL returned when ``state`` is absent, malformed, or fails verification. allowed_hosts: Open-redirect guard — the decoded destination must be a @@ -135,6 +137,10 @@ def oauth_decode_state( Returns: The destination URL embedded in ``state``, or ``fallback``. """ + # An empty expected token (e.g. session.get(..., "") on a session miss) + # must never match — compare_digest("", "") is True. + if not expected_state_token: + return fallback if not state or state == "null": # "null" guards against JS JSON.stringify(null) return fallback try: diff --git a/tests/test_multiauth.py b/tests/test_multiauth.py index 2572de3..1ffaa3e 100644 --- a/tests/test_multiauth.py +++ b/tests/test_multiauth.py @@ -1597,6 +1597,14 @@ def test_decode_wrong_state_token_returns_fallback(self): == "/" ) + def test_decode_empty_expected_token_returns_fallback(self): + """An empty stored token (session miss) must never match an empty "n".""" + encoded = oauth_encode_state("https://evil.example.com/x", "") + assert ( + oauth_decode_state(encoded, expected_state_token="", fallback="/home") + == "/home" + ) + def test_generate_state_token_is_random(self): assert oauth_generate_state_token() != oauth_generate_state_token() @@ -2269,6 +2277,17 @@ def test_backslash_path_rejected(self): """Browsers normalize "/\\evil.com" into a scheme-relative redirect.""" assert self._decode("/\\evil.example.com", ("app.example.com",)) == "/fallback" + def test_tab_path_rejected(self): + """Browsers strip tabs/newlines, turning "/\\t/evil.com" into "//evil.com".""" + assert self._decode("/\t/evil.example.com", ("app.example.com",)) == ( + "/fallback" + ) + + def test_newline_path_rejected(self): + assert self._decode("/\n/evil.example.com", ("app.example.com",)) == ( + "/fallback" + ) + def test_non_http_scheme_rejected(self): assert self._decode("javascript:alert(1)", ("app.example.com",)) == "/fallback"