Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/routes/auth/login/+page.server.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { redirect } from '@sveltejs/kit';
import type { PageServerLoad } from './$types';
import { safeRedirect } from './safe-redirect';

export const load: PageServerLoad = async ({ locals, url }) => {
if (locals.session?.userType === 'org') {
redirect(303, url.searchParams.get('redirect') ?? '/portal/dashboard');
redirect(303, safeRedirect(url.searchParams.get('redirect')));
}
};
6 changes: 4 additions & 2 deletions src/routes/auth/login/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@
import { goto } from '$app/navigation';
import { page } from '$app/state';
import { resolve } from '$app/paths';
import { safeRedirect } from './safe-redirect';
import type { PageData } from './$types';

let { data }: { data: PageData } = $props();

const redirectTo = $derived(page.url.searchParams.get('redirect') ?? '/portal/dashboard');
const redirectTo = $derived(page.url.searchParams.get('redirect'));

function handleSuccess() {
goto(resolve(redirectTo as '/portal/dashboard'));
// The `redirect` param is attacker-controlled — validate it before navigating.
goto(resolve(safeRedirect(redirectTo) as '/portal/dashboard'));
}
</script>

Expand Down
22 changes: 22 additions & 0 deletions src/routes/auth/login/safe-redirect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// The post-login `redirect` query param is attacker-controlled, so it must
// never be handed to `redirect()`/`goto()` without validation — otherwise it
// is an open redirect (an attacker can send `?redirect=https://evil.example`
// or the protocol-relative `?redirect=//evil.example` to bounce a logged-in
// user off-site). Only same-origin, path-absolute targets are allowed.

export const DEFAULT_REDIRECT = '/portal/dashboard';

/**
* Return a safe post-login redirect target. A value is only accepted when it
* begins with a single `/` (a same-origin absolute path). Everything else —
* empty/missing values, absolute URLs (`https://…`), protocol-relative URLs
* (`//host`) and the backslash variant browsers normalise to them (`/\host`)
* — falls back to {@link DEFAULT_REDIRECT}.
*/
export function safeRedirect(target: string | null | undefined): string {
if (!target || !target.startsWith('/')) return DEFAULT_REDIRECT;
Comment thread
dobby-coder[bot] marked this conversation as resolved.
Outdated
// Reject protocol-relative URLs. Browsers treat `\` as `/`, so `/\host`
// escapes the origin just like `//host` does.
if (target[1] === '/' || target[1] === '\\') return DEFAULT_REDIRECT;
return target;
}
35 changes: 35 additions & 0 deletions tests/unit/safe-redirect.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, it, expect } from 'vitest';
import { safeRedirect, DEFAULT_REDIRECT } from '../../src/routes/auth/login/safe-redirect';

describe('safeRedirect', () => {
it('accepts same-origin absolute paths', () => {
expect(safeRedirect('/portal/dashboard')).toBe('/portal/dashboard');
expect(safeRedirect('/portal/settings')).toBe('/portal/settings');
expect(safeRedirect('/')).toBe('/');
expect(safeRedirect('/a?b=c#d')).toBe('/a?b=c#d');
});

it('falls back for missing values', () => {
expect(safeRedirect(null)).toBe(DEFAULT_REDIRECT);
expect(safeRedirect(undefined)).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('')).toBe(DEFAULT_REDIRECT);
});

it('rejects values that do not start with a slash', () => {
expect(safeRedirect('portal/dashboard')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('https://evil.example')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('http://evil.example/path')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('javascript:alert(1)')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect(' /portal/dashboard')).toBe(DEFAULT_REDIRECT);
});

it('rejects protocol-relative URLs', () => {
expect(safeRedirect('//evil.example')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('//evil.example/path')).toBe(DEFAULT_REDIRECT);
});

it('rejects the backslash variant browsers normalise to protocol-relative', () => {
expect(safeRedirect('/\\evil.example')).toBe(DEFAULT_REDIRECT);
expect(safeRedirect('/\\/evil.example')).toBe(DEFAULT_REDIRECT);
});
});
Loading