From 3e546d4a1e312d320c2b9f518cb36de5f5da2e40 Mon Sep 17 00:00:00 2001 From: XShine555 Date: Thu, 24 Sep 2026 04:58:04 +0200 Subject: [PATCH] web-player: fix open redirect via returnTo, require SESSION_SECRET and validate API inputs --- web-player/.env.example | 5 +++-- web-player/src/lib/config.ts | 1 + web-player/src/lib/server/auth.test.ts | 21 +++++++++++++++++++ web-player/src/lib/server/auth.ts | 7 +++++++ web-player/src/lib/server/config.ts | 9 +++++++- web-player/src/lib/server/image.ts | 4 +++- web-player/src/routes/api/likes/+server.ts | 8 +++++-- .../api/listens/[id]/progress/+server.ts | 3 ++- web-player/src/routes/api/tracks/+server.ts | 7 +++++-- web-player/src/routes/auth/+page.server.ts | 6 ++++-- .../src/routes/auth/callback/+server.ts | 3 ++- web-player/src/routes/auth/login/+server.ts | 3 ++- 12 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 web-player/src/lib/server/auth.test.ts diff --git a/web-player/.env.example b/web-player/.env.example index 75b940b..d1f1cca 100644 --- a/web-player/.env.example +++ b/web-player/.env.example @@ -11,8 +11,9 @@ ZITADEL_CLIENT_SECRET=your-web-app-client-secret AUTH_REDIRECT_URI=http://localhost:5173/auth/callback AUTH_POST_LOGOUT_URI=http://localhost:5173/ -# Secret used to derive the session cookie encryption key (any long random string) -SESSION_SECRET=change-me-to-a-long-random-string +# Secret used to derive the session cookie encryption key. Required, at least +# 32 characters (e.g. `openssl rand -base64 48`). +SESSION_SECRET=change-me-to-a-long-random-string-of-32-chars-or-more # Backend API base URL API_BASE_URL=http://localhost:5111 diff --git a/web-player/src/lib/config.ts b/web-player/src/lib/config.ts index 8b94487..1369cf9 100644 --- a/web-player/src/lib/config.ts +++ b/web-player/src/lib/config.ts @@ -1,4 +1,5 @@ export const EXPLORE_PAGE_SIZE = 27; +export const MAX_PAGE_SIZE = 100; export const SEARCH_DEBOUNCE_MS = 450; export const SEARCH_MIN_LENGTH = 2; export const INFINITE_SCROLL_ROOT_MARGIN = '300px'; diff --git a/web-player/src/lib/server/auth.test.ts b/web-player/src/lib/server/auth.test.ts new file mode 100644 index 0000000..2a203ee --- /dev/null +++ b/web-player/src/lib/server/auth.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; +import { safeReturnTo } from './auth'; + +describe('safeReturnTo', () => { + it('keeps local paths', () => { + expect(safeReturnTo('/')).toBe('/'); + expect(safeReturnTo('/playlists/1?tab=x#top')).toBe('/playlists/1?tab=x#top'); + }); + it('falls back to / for missing values', () => { + expect(safeReturnTo(null)).toBe('/'); + expect(safeReturnTo(undefined)).toBe('/'); + expect(safeReturnTo('')).toBe('/'); + }); + it('rejects absolute and protocol-relative URLs', () => { + expect(safeReturnTo('https://example.com')).toBe('/'); + expect(safeReturnTo('//example.com')).toBe('/'); + expect(safeReturnTo('/\\example.com')).toBe('/'); + expect(safeReturnTo('javascript:alert(1)')).toBe('/'); + expect(safeReturnTo('relative/path')).toBe('/'); + }); +}); diff --git a/web-player/src/lib/server/auth.ts b/web-player/src/lib/server/auth.ts index 58f4a7a..7df84c1 100644 --- a/web-player/src/lib/server/auth.ts +++ b/web-player/src/lib/server/auth.ts @@ -18,6 +18,13 @@ export interface Session extends SessionUser { expiresAt: number; } +export function safeReturnTo(value: string | null | undefined): string { + if (!value || !value.startsWith('/') || value.startsWith('//') || value.startsWith('/\\')) { + return '/'; + } + return value; +} + let configPromise: Promise | undefined; export function getOidcConfig(): Promise { configPromise ??= (async () => { diff --git a/web-player/src/lib/server/config.ts b/web-player/src/lib/server/config.ts index 6f1420c..76ec9c6 100644 --- a/web-player/src/lib/server/config.ts +++ b/web-player/src/lib/server/config.ts @@ -1,6 +1,7 @@ import { env } from '$env/dynamic/private'; const DAY_SECONDS = 60 * 60 * 24; +const MIN_SESSION_SECRET_LENGTH = 32; const DEFAULT_OIDC_SCOPE = 'openid profile email offline_access'; function positiveInt(value: string | undefined, fallback: number): number { @@ -35,7 +36,13 @@ export const authConfig = { return required('AUTH_POST_LOGOUT_URI', env.AUTH_POST_LOGOUT_URI); }, get sessionSecret() { - return env.SESSION_SECRET; + const secret = required('SESSION_SECRET', env.SESSION_SECRET); + if (secret.length < MIN_SESSION_SECRET_LENGTH) { + throw new Error( + `SESSION_SECRET must be at least ${MIN_SESSION_SECRET_LENGTH} characters long.` + ); + } + return secret; }, get sessionTtlSeconds() { return positiveInt(env.SESSION_TTL_SECONDS, 7 * DAY_SECONDS); diff --git a/web-player/src/lib/server/image.ts b/web-player/src/lib/server/image.ts index 1f09eca..95d1900 100644 --- a/web-player/src/lib/server/image.ts +++ b/web-player/src/lib/server/image.ts @@ -10,7 +10,9 @@ export async function proxyCoverImage( ): Promise { const safeSize = size && ALLOWED_SIZES.has(size) ? size : 'medium'; - const upstream = await fetch(`${apiConfig.baseUrl}/${kind}/${id}/cover?size=${safeSize}`); + const upstream = await fetch( + `${apiConfig.baseUrl}/${kind}/${encodeURIComponent(id)}/cover?size=${safeSize}` + ); if (!upstream.ok) { return new Response(null, { status: upstream.status }); diff --git a/web-player/src/routes/api/likes/+server.ts b/web-player/src/routes/api/likes/+server.ts index 6faba85..836c30c 100644 --- a/web-player/src/routes/api/likes/+server.ts +++ b/web-player/src/routes/api/likes/+server.ts @@ -5,9 +5,13 @@ import { createApiClient } from '$lib/server/api'; export const POST: RequestHandler = async ({ request, locals, fetch }) => { if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 }); - const body = await request.json(); + const payload = await request.json().catch(() => null); + const trackId = payload?.trackId; + if (typeof trackId !== 'string' || trackId === '') + return json({ message: 'Canción inválida.' }, { status: 400 }); + const api = createApiClient({ fetch, accessToken: locals.accessToken }); - const result = await api.POST('/likes/toggle', { body }); + const result = await api.POST('/likes/toggle', { body: { trackId } }); if (result.error || result.data === undefined) return json({ message: 'No se pudo actualizar Me gusta.' }, { status: 502 }); diff --git a/web-player/src/routes/api/listens/[id]/progress/+server.ts b/web-player/src/routes/api/listens/[id]/progress/+server.ts index 78e4c9b..7cf9e67 100644 --- a/web-player/src/routes/api/listens/[id]/progress/+server.ts +++ b/web-player/src/routes/api/listens/[id]/progress/+server.ts @@ -5,7 +5,8 @@ import { createApiClient } from '$lib/server/api'; export const PUT: RequestHandler = async ({ params, request, locals, fetch }) => { if (!locals.accessToken) return json({ message: 'Inicia sesión.' }, { status: 401 }); - const { playedSeconds } = (await request.json()) as { playedSeconds?: number }; + const payload = (await request.json().catch(() => null)) as { playedSeconds?: unknown } | null; + const playedSeconds = payload?.playedSeconds; if (typeof playedSeconds !== 'number' || !Number.isFinite(playedSeconds) || playedSeconds < 0) return json({ message: 'Tiempo inválido.' }, { status: 400 }); diff --git a/web-player/src/routes/api/tracks/+server.ts b/web-player/src/routes/api/tracks/+server.ts index a460898..786796b 100644 --- a/web-player/src/routes/api/tracks/+server.ts +++ b/web-player/src/routes/api/tracks/+server.ts @@ -1,13 +1,16 @@ import type { RequestHandler } from './$types'; import { json } from '@sveltejs/kit'; import { createApiClient, unwrapOrError } from '$lib/server/api'; -import { EXPLORE_PAGE_SIZE } from '$lib/config'; +import { EXPLORE_PAGE_SIZE, MAX_PAGE_SIZE } from '$lib/config'; export const GET: RequestHandler = async ({ url, locals, fetch }) => { const name = url.searchParams.get('name')?.trim() || undefined; const genre = url.searchParams.get('genre')?.trim() || undefined; const pageNumber = Math.max(1, Number(url.searchParams.get('pageNumber')) || 1); - const pageSize = Math.max(1, Number(url.searchParams.get('pageSize')) || EXPLORE_PAGE_SIZE); + const pageSize = Math.min( + MAX_PAGE_SIZE, + Math.max(1, Number(url.searchParams.get('pageSize')) || EXPLORE_PAGE_SIZE) + ); const api = createApiClient({ fetch, accessToken: locals.accessToken ?? undefined }); const result = await api.GET('/tracks', { diff --git a/web-player/src/routes/auth/+page.server.ts b/web-player/src/routes/auth/+page.server.ts index 13f45cd..6b37d11 100644 --- a/web-player/src/routes/auth/+page.server.ts +++ b/web-player/src/routes/auth/+page.server.ts @@ -1,9 +1,11 @@ import { redirect } from '@sveltejs/kit'; import type { PageServerLoad } from './$types'; +import { safeReturnTo } from '$lib/server/auth'; export const load: PageServerLoad = ({ locals, url }) => { + const returnTo = safeReturnTo(url.searchParams.get('returnTo')); if (locals.user) { - redirect(302, url.searchParams.get('returnTo') ?? '/'); + redirect(302, returnTo); } - return { returnTo: url.searchParams.get('returnTo') ?? '/' }; + return { returnTo }; }; diff --git a/web-player/src/routes/auth/callback/+server.ts b/web-player/src/routes/auth/callback/+server.ts index ea7c909..97ac6f7 100644 --- a/web-player/src/routes/auth/callback/+server.ts +++ b/web-player/src/routes/auth/callback/+server.ts @@ -5,6 +5,7 @@ import { getOidcConfig, encodeSession, toSessionUser, + safeReturnTo, writeSessionCookie, type Session, STATE_COOKIE, @@ -17,7 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => { const codeVerifier = cookies.get(VERIFIER_COOKIE); const state = cookies.get(STATE_COOKIE); const nonce = cookies.get(NONCE_COOKIE); - const returnTo = cookies.get(RETURN_COOKIE) ?? '/'; + const returnTo = safeReturnTo(cookies.get(RETURN_COOKIE)); if (!codeVerifier || !state) { error(400, 'Estado de autenticación inválido. Vuelve a iniciar sesión.'); diff --git a/web-player/src/routes/auth/login/+server.ts b/web-player/src/routes/auth/login/+server.ts index 45632f5..f56c614 100644 --- a/web-player/src/routes/auth/login/+server.ts +++ b/web-player/src/routes/auth/login/+server.ts @@ -3,6 +3,7 @@ import * as client from 'openid-client'; import type { RequestHandler } from './$types'; import { getOidcConfig, + safeReturnTo, STATE_COOKIE, VERIFIER_COOKIE, NONCE_COOKIE, @@ -19,7 +20,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => { const nonce = client.randomNonce(); const register = url.searchParams.get('mode') === 'register'; - const returnTo = url.searchParams.get('returnTo') ?? '/'; + const returnTo = safeReturnTo(url.searchParams.get('returnTo')); const parameters: Record = { redirect_uri: authConfig.redirectUri,