From df680f6226b26690e2f7e31fff3fc97a9ab70555 Mon Sep 17 00:00:00 2001 From: mwiegand Date: Wed, 6 May 2026 13:24:04 +0200 Subject: [PATCH] fix(l4d2-web): reject encoded unsafe redirects --- .../plans/2026-05-06-l4d2-web-auth-pages.md | 31 ++++++++++++++++++- .../2026-05-06-l4d2-web-auth-pages-design.md | 2 ++ l4d2web/auth.py | 9 +++++- l4d2web/tests/test_auth.py | 22 +++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) 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 315afaa..cb00d98 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 @@ -44,6 +44,15 @@ def test_login_page_renders_form(client) -> None: assert "signup" not in text.lower() +def test_login_page_drops_unsafe_encoded_next(client) -> None: + response = client.get("/login?next=/%5Cevil.com") + text = response.get_data(as_text=True) + + assert response.status_code == 200 + assert 'name="next"' not in text + assert "evil.com" not in text + + def test_signup_routes_are_gone(client) -> None: assert client.get("/signup").status_code == 404 assert client.post("/signup", data={"username": "alice", "password": "secret"}).status_code == 404 @@ -86,6 +95,19 @@ def test_login_ignores_backslash_next(client) -> None: assert response.status_code == 302 assert response.headers["Location"].endswith("/dashboard") + + +def test_login_ignores_percent_encoded_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": "/%5Cevil.com"}, + ) + + assert response.status_code == 302 + assert response.headers["Location"].endswith("/dashboard") ``` - [ ] **Step 2: Add protected-page and root redirect tests** @@ -149,7 +171,7 @@ Expected: FAIL because `/login` is still plain text, signup routes still exist, Update `l4d2web/auth.py` imports and add helpers near the session helpers: ```python -from urllib.parse import quote +from urllib.parse import quote, unquote from flask import abort, g, redirect, request, session ``` @@ -166,6 +188,13 @@ def is_safe_next(target: str | None) -> bool: return False if "\\" in target: return False + decoded_target = unquote(target) + if decoded_target.startswith("//"): + return False + if "://" in decoded_target: + return False + if "\\" in decoded_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 0140527..e36afeb 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 @@ -47,6 +47,7 @@ A `next` target is safe only when it is a local absolute path: - 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. +- The same checks apply after one percent-decoding pass so encoded backslashes and encoded protocol-relative paths are rejected. Unsafe values are ignored and replaced with `/dashboard`. This avoids open redirects while keeping the implementation simple. @@ -87,6 +88,7 @@ Tests should cover: - 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`. +- A successful login with a percent-encoded backslash in `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 a2f21ff..74688f4 100644 --- a/l4d2web/auth.py +++ b/l4d2web/auth.py @@ -1,6 +1,6 @@ from functools import wraps from typing import Callable, TypeVar -from urllib.parse import quote +from urllib.parse import quote, unquote from flask import abort, g, redirect, request, session from sqlalchemy import select @@ -53,6 +53,13 @@ def is_safe_next(target: str | None) -> bool: return False if "\\" in target: return False + decoded_target = unquote(target) + if decoded_target.startswith("//"): + return False + if "://" in decoded_target: + return False + if "\\" in decoded_target: + return False return True diff --git a/l4d2web/tests/test_auth.py b/l4d2web/tests/test_auth.py index c567451..4985ced 100644 --- a/l4d2web/tests/test_auth.py +++ b/l4d2web/tests/test_auth.py @@ -41,6 +41,15 @@ def test_login_page_renders_form(client) -> None: assert "signup" not in text.lower() +def test_login_page_drops_unsafe_encoded_next(client) -> None: + response = client.get("/login?next=/%5Cevil.com") + text = response.get_data(as_text=True) + + assert response.status_code == 200 + assert 'name="next"' not in text + assert "evil.com" not in text + + def test_signup_routes_are_gone(client) -> None: assert client.get("/signup").status_code == 404 assert client.post("/signup", data={"username": "alice", "password": "secret"}).status_code == 404 @@ -85,6 +94,19 @@ def test_login_ignores_backslash_next(client) -> None: assert response.headers["Location"].endswith("/dashboard") +def test_login_ignores_percent_encoded_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": "/%5Cevil.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))