diff --git a/docs/superpowers/plans/2026-05-06-l4d2-web-auth-pages.md b/docs/superpowers/plans/2026-05-06-l4d2-web-auth-pages.md index c37a5ec..315afaa 100644 --- a/docs/superpowers/plans/2026-05-06-l4d2-web-auth-pages.md +++ b/docs/superpowers/plans/2026-05-06-l4d2-web-auth-pages.md @@ -73,6 +73,19 @@ def test_login_ignores_unsafe_next(client) -> None: assert response.status_code == 302 assert response.headers["Location"].endswith("/dashboard") + + +def test_login_ignores_backslash_next(client) -> None: + with session_scope() as session: + session.add(User(username="alice", password_digest=hash_password("secret"), admin=False)) + + response = client.post( + "/login", + data={"username": "alice", "password": "secret", "next": "/\\evil.com"}, + ) + + assert response.status_code == 302 + assert response.headers["Location"].endswith("/dashboard") ``` - [ ] **Step 2: Add protected-page and root redirect tests** @@ -151,6 +164,8 @@ def is_safe_next(target: str | None) -> bool: return False if "://" in target: return False + if "\\" in target: + return False return True diff --git a/docs/superpowers/specs/2026-05-06-l4d2-web-auth-pages-design.md b/docs/superpowers/specs/2026-05-06-l4d2-web-auth-pages-design.md index 5278f39..0140527 100644 --- a/docs/superpowers/specs/2026-05-06-l4d2-web-auth-pages-design.md +++ b/docs/superpowers/specs/2026-05-06-l4d2-web-auth-pages-design.md @@ -46,6 +46,7 @@ A `next` target is safe only when it is a local absolute path: - It must start with `/`. - It must not start with `//`. - It must not contain a URL scheme such as `https://`. +- It must not contain backslashes, which browsers may normalize into path separators. Unsafe values are ignored and replaced with `/dashboard`. This avoids open redirects while keeping the implementation simple. @@ -85,6 +86,7 @@ Tests should cover: - Anonymous protected pages redirect to `/login?next=`. - A successful login with a safe `next` redirects to that target. - A successful login with an unsafe `next` redirects to `/dashboard`. +- A successful login with a backslash-containing `next` redirects to `/dashboard`. - Anonymous `/` redirects to `/login`. - Logged-in `/` redirects to `/dashboard`. - Non-admin users still receive `403` for admin pages. diff --git a/l4d2web/auth.py b/l4d2web/auth.py index 9abf773..a2f21ff 100644 --- a/l4d2web/auth.py +++ b/l4d2web/auth.py @@ -51,6 +51,8 @@ def is_safe_next(target: str | None) -> bool: return False if "://" in target: return False + if "\\" in target: + return False return True diff --git a/l4d2web/tests/test_auth.py b/l4d2web/tests/test_auth.py index ce7c70f..c567451 100644 --- a/l4d2web/tests/test_auth.py +++ b/l4d2web/tests/test_auth.py @@ -72,6 +72,19 @@ def test_login_ignores_unsafe_next(client) -> None: assert response.headers["Location"].endswith("/dashboard") +def test_login_ignores_backslash_next(client) -> None: + with session_scope() as session: + session.add(User(username="alice", password_digest=hash_password("secret"), admin=False)) + + response = client.post( + "/login", + data={"username": "alice", "password": "secret", "next": "/\\evil.com"}, + ) + + assert response.status_code == 302 + assert response.headers["Location"].endswith("/dashboard") + + def test_login_sets_session(client) -> None: with session_scope() as session: session.add(User(username="alice", password_digest=hash_password("secret"), admin=False))