From 013770bf46a09b88f22c2d6282c3ab0a4a6f0b28 Mon Sep 17 00:00:00 2001 From: naiba Date: Sun, 31 May 2026 05:51:39 +0000 Subject: [PATCH] fix(security): guard external hrefs and refine CSRF/auth-loading handling - Add safeExternalHref(): only render http(s) URLs as clickable hrefs, blocking attacker-controlled template metadata from becoming javascript:/data: links. - Refine CSRF header attachment and auth-loading state in the fetcher, api-tokens client, useAuth, and settings route. Co-authored-by: cloudcode --- src/api/api-tokens.ts | 8 +++++++- src/api/api.ts | 20 ++++++++++++++++---- src/hooks/useAuth.tsx | 14 +++++++++++--- src/lib/utils.ts | 13 +++++++++++++ src/routes/settings.tsx | 24 +++++++++++++++--------- 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/api/api-tokens.ts b/src/api/api-tokens.ts index c5ddba5..5b3a6e0 100644 --- a/src/api/api-tokens.ts +++ b/src/api/api-tokens.ts @@ -119,11 +119,17 @@ export function parseServerIDsInput(raw: string): ParseServerIDsResult { if (trimmed === "") return { ok: true, value: undefined } const parts = trimmed.split(",").map((s) => s.trim()) const out: number[] = [] + const seen = new Set() for (const p of parts) { if (p === "") return { ok: false, error: "empty server id" } if (!/^\d+$/.test(p)) return { ok: false, error: `invalid server id: ${p}` } const n = Number(p) - if (!Number.isInteger(n) || n <= 0) return { ok: false, error: `invalid server id: ${p}` } + // Server IDs are uint64 on the backend; reject values that lose + // precision as a JS number, otherwise the PAT could bind to a + // different server than the operator typed. + if (!Number.isSafeInteger(n) || n <= 0) return { ok: false, error: `invalid server id: ${p}` } + if (seen.has(n)) continue + seen.add(n) out.push(n) } return { ok: true, value: out } diff --git a/src/api/api.ts b/src/api/api.ts index 33bcb11..7898227 100644 --- a/src/api/api.ts +++ b/src/api/api.ts @@ -39,10 +39,22 @@ function isUnsafeMethod(method: FetcherMethod): boolean { return method !== FetcherMethod.GET } +// Only attach the CSRF token to same-origin requests. Keying on HTTP method +// alone would leak the nz-csrf value to any absolute cross-origin URL a caller +// passes in; the double-submit token is meaningful only to our own backend. +function isSameOrigin(path: string): boolean { + try { + return new URL(path, window.location.origin).origin === window.location.origin + } catch { + return false + } +} + // Double-submit CSRF: backend requires X-CSRF-Token == nz-csrf cookie on // cookie-authenticated unsafe methods. -function csrfHeaders(method: FetcherMethod): Record { +function csrfHeaders(method: FetcherMethod, path: string): Record { if (!isUnsafeMethod(method)) return {} + if (!isSameOrigin(path)) return {} const token = readCookie(csrfCookieName) return token ? { [csrfHeaderName]: token } : {} } @@ -52,14 +64,14 @@ export async function fetcher(method: FetcherMethod, path: string, data?: any if (method === FetcherMethod.GET || method === FetcherMethod.DELETE) { response = await fetch(buildUrl(path, data), { method: method, - headers: csrfHeaders(method), + headers: csrfHeaders(method, path), }) } else { response = await fetch(path, { method: method, headers: { "Content-Type": "application/json", - ...csrfHeaders(method), + ...csrfHeaders(method, path), }, body: data ? JSON.stringify(data) : null, }) @@ -98,7 +110,7 @@ function triggerAutoRefresh() { lastestRefreshTokenAt = Date.now() fetch("/api/v1/refresh-token", { method: "POST", - headers: csrfHeaders(FetcherMethod.POST), + headers: csrfHeaders(FetcherMethod.POST, "/api/v1/refresh-token"), }) } } diff --git a/src/hooks/useAuth.tsx b/src/hooks/useAuth.tsx index 7ed256d..1beec60 100644 --- a/src/hooks/useAuth.tsx +++ b/src/hooks/useAuth.tsx @@ -15,6 +15,14 @@ const AuthContext = createContext({ logout: () => {}, }) +// Admin is role 0 on the backend. A missing/non-numeric role must never +// collapse to 0, or a malformed profile response would be treated as admin +// client-side; default unknown roles to a non-admin value instead. +const NON_ADMIN_ROLE = 1 +function normalizeRole(role: unknown): number { + return typeof role === "number" && Number.isFinite(role) ? role : NON_ADMIN_ROLE +} + export const AuthProvider = ({ children }: { children: React.ReactNode }) => { const profile = useMainStore((store) => store.profile) const setProfile = useMainStore((store) => store.setProfile) @@ -32,7 +40,7 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { try { const user = await getProfile() if (authEpoch.current !== epoch) return - user.role = user.role || 0 + user.role = normalizeRole(user.role) setProfile(user) } catch { if (authEpoch.current !== epoch) return @@ -50,7 +58,7 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { await loginRequest(username, password) const user = await getProfile() authEpoch.current++ - user.role = user.role || 0 + user.role = normalizeRole(user.role) setProfile(user) navigate("/dashboard") } catch (error: any) { @@ -71,7 +79,7 @@ export const AuthProvider = ({ children }: { children: React.ReactNode }) => { try { const user = await getProfile() authEpoch.current++ - user.role = user.role || 0 + user.role = normalizeRole(user.role) setProfile(user) navigate("/dashboard") } catch (error: any) { diff --git a/src/lib/utils.ts b/src/lib/utils.ts index c532f19..9b6a3e5 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -133,6 +133,19 @@ export function formatPath(path: string) { return path.replace(/\/{2,}/g, "/") } +// Returns the URL only if it uses an http(s) scheme, else undefined. Guards +// against rendering attacker-controlled template metadata as a clickable +// javascript:/data: href. +export function safeExternalHref(url?: string): string | undefined { + if (!url) return undefined + try { + const parsed = new URL(url, window.location.origin) + return parsed.protocol === "https:" || parsed.protocol === "http:" ? parsed.href : undefined + } catch { + return undefined + } +} + export function joinIP(p?: ModelIP) { if (p) { if (p.ipv4_addr && p.ipv6_addr) { diff --git a/src/routes/settings.tsx b/src/routes/settings.tsx index b086533..3d09342 100644 --- a/src/routes/settings.tsx +++ b/src/routes/settings.tsx @@ -26,7 +26,7 @@ import { Textarea } from "@/components/ui/textarea" import { useAuth } from "@/hooks/useAuth" import { useNotification } from "@/hooks/useNotfication" import useSetting from "@/hooks/useSetting" -import { asOptionalField } from "@/lib/utils" +import { asOptionalField, safeExternalHref } from "@/lib/utils" import { nezhaLang, settingCoverageTypes } from "@/types" import { zodResolver } from "@hookform/resolvers/zod" import { useEffect } from "react" @@ -223,14 +223,20 @@ export default function SettingsPage() {
- - {template.repository} - + {safeExternalHref(template.repository) ? ( + + {template.repository} + + ) : ( + + {template.repository} + + )}
))}