diff --git a/tradein-mvp/frontend/src/app/login/page.tsx b/tradein-mvp/frontend/src/app/login/page.tsx index 4d3965dc..b3529e83 100644 --- a/tradein-mvp/frontend/src/app/login/page.tsx +++ b/tradein-mvp/frontend/src/app/login/page.tsx @@ -43,11 +43,31 @@ function readNextParam(): string | null { * Open-redirect guard: принимаем только внутренний путь, начинающийся * ровно с одного "/" — не "//host" (protocol-relative URL) и не "/\host" * (браузеры местами трактуют backslash как forward slash в URL-парсинге). + * + * PR #2562 review finding 1: WHATWG URL-парсер (который `router.push` + * использует под капотом) убирает ВСЕ ASCII tab/CR/LF из строки ПЕРЕД + * парсингом — так `"/\t//evil"` для наивного regex выглядит как безопасный + * путь с одним leading slash (символ в позиции 1 — таб, не "/" и не "\"), + * а после навигации превращается в `"//evil"` (protocol-relative → чужой + * origin). Убираем те же символы ДО валидации, чтобы regex видел ту же + * строку, что увидит парсер. + * + * PR #2562 review finding 2: `next=/login` (или `/login?...`) после успешного + * логина кидал бы юзера обратно на форму входа (RouteGuard не гейтит + * `/login`) — dead-end. Фолбэк на "/" в этом случае. */ function sanitizeNext(next: string | null): string { if (!next) return "/"; - if (!/^\/(?!\/|\\)/.test(next)) return "/"; - return next; + const cleaned = next.replace(/[\t\r\n]/g, ""); + if (!/^\/(?!\/|\\)/.test(cleaned)) return "/"; + if ( + cleaned === "/login" || + cleaned.startsWith("/login?") || + cleaned.startsWith("/login#") + ) { + return "/"; + } + return cleaned; } function loginErrorMessage(error: unknown): string { diff --git a/tradein-mvp/frontend/src/components/auth/RouteGuard.tsx b/tradein-mvp/frontend/src/components/auth/RouteGuard.tsx index 7f21c7ab..513d4310 100644 --- a/tradein-mvp/frontend/src/components/auth/RouteGuard.tsx +++ b/tradein-mvp/frontend/src/components/auth/RouteGuard.tsx @@ -61,7 +61,14 @@ export function RouteGuard({ children }: RouteGuardProps) { useEffect(() => { if (!shouldRedirectToLogin) return; - router.push(`/login?next=${encodeURIComponent(rawPath)}`); + // PR #2562 review finding 3: deep-links carry их state в query (`/v2?id= + // ` — см. next.config.ts redirect comment про restore-by-id). Без + // `window.location.search` юзер, чья сессия истекла mid-session на такой + // ссылке, после логина попадал бы на голый `/v2` и терял отчёт. Effect + // — гарантированно client-side (useEffect тело никогда не бежит на SSR), + // поэтому `window` тут безопасен без typeof-guard. + const next = `${rawPath}${window.location.search}`; + router.push(`/login?next=${encodeURIComponent(next)}`); }, [shouldRedirectToLogin, rawPath, router]); // #801: preview-страница самодостаточна (свой QueryClient с фейковым me),