fix(tradein/ui): харденинг sanitizeNext + сохранение query в next (#2555)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m1s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m1s
PR #2562 review, 3 однострочника: 1. sanitizeNext обходился: WHATWG URL-парсер (router.push) вырезает ASCII tab/CR/LF из ВСЕЙ строки перед парсингом, так что "/\t//evil" проходил regex (позиция 1 — таб, не "/"/"\\"), а после навигации резолвился в protocol-relative "//evil" → чужой origin. Теперь сначала strip [\t\r\n], потом валидация — regex видит ту же строку, что увидит парсер. 2. next=/login (или /login?...) кидал юзера обратно на форму входа (RouteGuard не гейтит /login) — dead-end. Фолбэк на "/". 3. RouteGuard брал next= только из usePathname(), без query — сессия, истёкшая на deep-link (/v2?id=<uuid>), теряла отчёт после релогина. Добавлен window.location.search в next (effect всегда client-side).
This commit is contained in:
parent
15d506b7dd
commit
e6d68c349e
2 changed files with 30 additions and 3 deletions
|
|
@ -43,11 +43,31 @@ function readNextParam(): string | null {
|
||||||
* Open-redirect guard: принимаем только внутренний путь, начинающийся
|
* Open-redirect guard: принимаем только внутренний путь, начинающийся
|
||||||
* ровно с одного "/" — не "//host" (protocol-relative URL) и не "/\host"
|
* ровно с одного "/" — не "//host" (protocol-relative URL) и не "/\host"
|
||||||
* (браузеры местами трактуют backslash как forward slash в URL-парсинге).
|
* (браузеры местами трактуют 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 {
|
function sanitizeNext(next: string | null): string {
|
||||||
if (!next) return "/";
|
if (!next) return "/";
|
||||||
if (!/^\/(?!\/|\\)/.test(next)) return "/";
|
const cleaned = next.replace(/[\t\r\n]/g, "");
|
||||||
return next;
|
if (!/^\/(?!\/|\\)/.test(cleaned)) return "/";
|
||||||
|
if (
|
||||||
|
cleaned === "/login" ||
|
||||||
|
cleaned.startsWith("/login?") ||
|
||||||
|
cleaned.startsWith("/login#")
|
||||||
|
) {
|
||||||
|
return "/";
|
||||||
|
}
|
||||||
|
return cleaned;
|
||||||
}
|
}
|
||||||
|
|
||||||
function loginErrorMessage(error: unknown): string {
|
function loginErrorMessage(error: unknown): string {
|
||||||
|
|
|
||||||
|
|
@ -61,7 +61,14 @@ export function RouteGuard({ children }: RouteGuardProps) {
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!shouldRedirectToLogin) return;
|
if (!shouldRedirectToLogin) return;
|
||||||
router.push(`/login?next=${encodeURIComponent(rawPath)}`);
|
// PR #2562 review finding 3: deep-links carry их state в query (`/v2?id=
|
||||||
|
// <uuid>` — см. 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]);
|
}, [shouldRedirectToLogin, rawPath, router]);
|
||||||
|
|
||||||
// #801: preview-страница самодостаточна (свой QueryClient с фейковым me),
|
// #801: preview-страница самодостаточна (свой QueryClient с фейковым me),
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue