diff --git a/.changeset/some-groups-return.md b/.changeset/some-groups-return.md new file mode 100644 index 000000000000..a7ee2cb52233 --- /dev/null +++ b/.changeset/some-groups-return.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +breaking: forbid external redirects by default diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index ff1a02d834c5..f68ef11770d3 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -11,6 +11,7 @@ import { strip_resolution_suffix } from '../runtime/pathname.js'; import { text_encoder } from '../runtime/utils.js'; +import { validate_redirect_location } from '../utils/url.js'; export { VERSION } from '../version.js'; @@ -92,19 +93,23 @@ export function isHttpError(e, status) { * * @param {300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308 | ({} & number)} status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#redirection_messages). Must be in the range 300-308. * @param {string | URL} location The location to redirect to. + * @param {{ external?: boolean | string[] }} [options] To redirect to an external URL, you must pass `{ external: true }` to allow any external URL except `javascript:` URLs, or `{ external: [...] }` with an allowlist of permitted origins. * @throws {import('./public.js').Redirect} This error instructs SvelteKit to redirect to the specified location. - * @throws {Error} If the provided status is invalid or the location cannot be used as a header value. + * @throws {Error} If the provided status is invalid, the location cannot be used as a header value, or the location is an external URL without permission. * @return {never} */ -export function redirect(status, location) { +export function redirect(status, location, options) { if ((!BROWSER || DEV) && (isNaN(status) || status < 300 || status > 308)) { throw new Error('Invalid status code'); } + const href = location.toString(); + validate_redirect_location(href, options); + throw new Redirect( // @ts-ignore status, - location.toString() + href ); } diff --git a/packages/kit/src/exports/index.spec.js b/packages/kit/src/exports/index.spec.js index faac39323672..35575da47045 100644 --- a/packages/kit/src/exports/index.spec.js +++ b/packages/kit/src/exports/index.spec.js @@ -67,6 +67,90 @@ describe('redirect', () => { } }); + it('throws Redirect for external locations with external: true', () => { + try { + redirect(307, 'https://google.de', { external: true }); + assert.fail('Expected redirect to throw'); + } catch (e) { + if (!isRedirect(e)) { + assert.fail('Expected a Redirect error'); + } + + assert.equal(e.status, 307); + assert.equal(e.location, 'https://google.de'); + } + }); + + it('throws Redirect for allowlisted external locations', () => { + try { + redirect(307, 'https://google.de/search', { external: ['https://google.de'] }); + assert.fail('Expected redirect to throw'); + } catch (e) { + if (!isRedirect(e)) { + assert.fail('Expected a Redirect error'); + } + + assert.equal(e.status, 307); + assert.equal(e.location, 'https://google.de/search'); + } + }); + + it('throws a descriptive error for external redirect locations', () => { + assert.throws( + () => redirect(307, 'https://google.de'), + /Cannot redirect to external URL "https:\/\/google\.de"/ + ); + }); + + it('throws a descriptive error for redirect locations that parse as external', () => { + assert.throws( + () => redirect(307, ' https://google.de'), + /Cannot redirect to external URL " https:\/\/google\.de"/ + ); + + assert.throws( + () => redirect(307, '\\\\google.de'), + /Cannot redirect to external URL "\\\\\\\\google\.de"/ + ); + + assert.throws(() => redirect(307, 'x:foo'), /Cannot redirect to external URL "x:foo"/); + }); + + it('throws a descriptive error for javascript URLs with external: true', () => { + assert.throws( + () => redirect(307, 'javascript:alert(1)', { external: true }), + /Cannot redirect to "javascript:alert\(1\)" with `{ external: true }`/ + ); + }); + + it('throws a descriptive error for normalized javascript URLs with external: true', () => { + assert.throws( + () => redirect(307, 'java\tscript:alert(1)', { external: true }), + /Cannot redirect to "java\\tscript:alert\(1\)" with `{ external: true }`/ + ); + }); + + it('throws Redirect for allowlisted javascript URLs', () => { + try { + redirect(307, 'javascript:alert(1)', { external: ['javascript:'] }); + assert.fail('Expected redirect to throw'); + } catch (e) { + if (!isRedirect(e)) { + assert.fail('Expected a Redirect error'); + } + + assert.equal(e.status, 307); + assert.equal(e.location, 'javascript:alert(1)'); + } + }); + + it('throws a descriptive error for disallowed external locations', () => { + assert.throws( + () => redirect(307, 'https://evil.com', { external: ['https://google.de'] }), + /Cannot redirect to "https:\/\/evil\.com": URL origin is not included in the `external` allowlist/ + ); + }); + it('throws a descriptive error for invalid redirect locations', () => { assert.throws( () => redirect(307, '/invalid\r\nset-cookie: x=y'), diff --git a/packages/kit/src/utils/url.js b/packages/kit/src/utils/url.js index 04445d3a286b..a375cc4c3a78 100644 --- a/packages/kit/src/utils/url.js +++ b/packages/kit/src/utils/url.js @@ -1,10 +1,13 @@ import { BROWSER, DEV } from 'esm-env'; +import { try_get_request_store } from '../exports/internal/event.js'; /** * Matches a URI scheme. See https://www.rfc-editor.org/rfc/rfc3986#section-3.1 * @type {RegExp} */ -export const SCHEME = /^[a-z][a-z\d+\-.]+:/i; +export const SCHEME = /^[a-z][a-z\d+\-.]*:/i; +// See https://datatracker.ietf.org/doc/html/rfc2606 - no domains under the .invalid TLD can be registered +const REDIRECT_BASE = 'https://sveltekit-redirect.invalid'; const internal = new URL('a://'); @@ -27,6 +30,100 @@ export function is_root_relative(path) { return path[0] === '/' && path[1] !== '/'; } +/** + * Whether a redirect location is absolute, i.e. not a root-relative or path-relative URL, + * and not pointing to the same origin as we're currently on (if determineable). + * @param {string} location + */ +export function is_external_location(location) { + const origin = BROWSER ? window.location.origin : try_get_request_store()?.event.url.origin; + + try { + return !matches_external_allowlist_entry(location, origin ?? REDIRECT_BASE); + } catch { + return true; + } +} + +/** + * @param {string} location + */ +function is_javascript_location(location) { + try { + return new URL(location, REDIRECT_BASE).protocol === 'javascript:'; + } catch { + return false; + } +} + +/** + * @param {string} location + * @param {string} allowed + */ +export function matches_external_allowlist_entry(location, allowed) { + if (location === allowed) return true; + + try { + const allow = new URL(allowed); + const loc = new URL(location, allow); + + // this is stricter than `loc.origin === allow.origin`, which can fail in `blob:` cases + return loc.protocol === allow.protocol && loc.host === allow.host; + } catch { + return false; + } +} + +/** + * @param {string} location + * @param {{ external?: boolean | string[] }} [options] + */ +export function validate_redirect_location(location, options) { + if (!is_external_location(location)) return; + + const external = options?.external; + + if (!external) { + throw new Error( + DEV + ? `Cannot redirect to external URL ${JSON.stringify(location)}. ` + + 'To redirect to an external URL, pass `{ external: true }` or an allowlist of permitted origins as the third argument to `redirect`' + : 'Cannot redirect to external URL unless explicitly allowed' + ); + } + + if (external === true) { + if (is_javascript_location(location)) { + throw new Error( + DEV + ? `Cannot redirect to ${JSON.stringify(location)} with \`{ external: true }\`. ` + + 'The `:javascript` protocol must be explicitly listed in the `external` allowlist' + : 'Cannot redirect to external URL unless explicitly allowed' + ); + } + + return; + } + + if (Array.isArray(external)) { + if (!external.some((allowed) => matches_external_allowlist_entry(location, allowed))) { + throw new Error( + DEV + ? `Cannot redirect to ${JSON.stringify(location)}: URL origin is not included in the \`external\` allowlist` + : 'Cannot redirect to external URL unless explicitly allowed' + ); + } + + return; + } + + throw new Error( + DEV + ? '`redirect` options.external must be `true` or an array of allowed origins' + : 'Invalid redirect options.external value' + ); +} + /** * @param {string} path * @param {import('types').TrailingSlash} trailing_slash diff --git a/packages/kit/src/utils/url.spec.js b/packages/kit/src/utils/url.spec.js index 4ad5244833f2..4ea102c5aed8 100644 --- a/packages/kit/src/utils/url.spec.js +++ b/packages/kit/src/utils/url.spec.js @@ -1,5 +1,13 @@ import { assert, describe } from 'vitest'; -import { resolve, normalize_path, make_trackable, disable_search } from './url.js'; +import { + resolve, + normalize_path, + make_trackable, + disable_search, + is_external_location, + matches_external_allowlist_entry, + validate_redirect_location +} from './url.js'; describe('resolve', (test) => { test('resolves a root-relative path', () => { @@ -67,6 +75,83 @@ describe('resolve', (test) => { }); }); +describe('is_absolute_location', (test) => { + test('detects absolute URLs', () => { + assert.equal(is_external_location('https://example.com'), true); + assert.equal(is_external_location('//example.com/foo'), true); + assert.equal(is_external_location('mailto:hello@svelte.dev'), true); + assert.equal(is_external_location('javascript:alert(1)'), true); + assert.equal(is_external_location(' https://example.com'), true); + assert.equal(is_external_location('\thttps://example.com'), true); + assert.equal(is_external_location('java\tscript:alert(1)'), true); + assert.equal(is_external_location('\\\\example.com/foo'), true); + assert.equal(is_external_location('/\\\\example.com/foo'), true); + assert.equal(is_external_location('x:foo'), true); + assert.equal(is_external_location('blob:https://sveltekit-redirect.invalid/id'), true); + }); + + test('detects relative URLs', () => { + assert.equal(is_external_location('/foo'), false); + assert.equal(is_external_location('./foo'), false); + assert.equal(is_external_location('foo'), false); + assert.equal(is_external_location('#hash'), false); + assert.equal(is_external_location('?query'), false); + }); +}); + +describe('matches_external_allowlist_entry', (test) => { + test('matches allowed origins', () => { + assert.equal(matches_external_allowlist_entry('https://google.de', 'https://google.de'), true); + assert.equal( + matches_external_allowlist_entry('https://google.de/search', 'https://google.de'), + true + ); + assert.equal( + matches_external_allowlist_entry('https://google.de/news', 'https://google.de/search'), + true + ); + assert.equal( + matches_external_allowlist_entry('https://google.de.evil.com', 'https://google.de'), + false + ); + assert.equal( + matches_external_allowlist_entry('blob:https://google.de/id', 'https://google.de'), + false + ); + assert.equal(matches_external_allowlist_entry('https://evil.com', 'https://google.de'), false); + }); +}); + +describe('validate_redirect_location', (test) => { + test('allows relative locations without options', () => { + validate_redirect_location('/foo'); + }); + + test('requires permission for absolute locations', () => { + assert.throws( + () => validate_redirect_location('https://google.de'), + /Cannot redirect to external URL "https:\/\/google\.de"/ + ); + }); + + test('requires permission for locations that parse as absolute after URL normalization', () => { + assert.throws( + () => validate_redirect_location(' https://google.de'), + /Cannot redirect to external URL " https:\/\/google\.de"/ + ); + + assert.throws( + () => validate_redirect_location('\\\\google.de'), + /Cannot redirect to external URL "\\\\\\\\google\.de"/ + ); + + assert.throws( + () => validate_redirect_location('x:foo'), + /Cannot redirect to external URL "x:foo"/ + ); + }); +}); + describe('normalize_path', (test) => { test('normalizes paths', () => { /** @type {Record} */ diff --git a/packages/kit/test/prerendering/basics/src/routes/redirect-encoded/+page.js b/packages/kit/test/prerendering/basics/src/routes/redirect-encoded/+page.js index 0b5f120e8035..abd27ca9f03f 100644 --- a/packages/kit/test/prerendering/basics/src/routes/redirect-encoded/+page.js +++ b/packages/kit/test/prerendering/basics/src/routes/redirect-encoded/+page.js @@ -2,5 +2,7 @@ import { redirect } from '@sveltejs/kit'; /** @type {import('@sveltejs/kit').Load} */ export function load() { - redirect(301, `https://example.com/redirected?returnTo=${encodeURIComponent('/foo?bar=baz')}`); + redirect(301, `https://example.com/redirected?returnTo=${encodeURIComponent('/foo?bar=baz')}`, { + external: true + }); } diff --git a/packages/kit/test/prerendering/basics/src/routes/redirect-malicious/+page.js b/packages/kit/test/prerendering/basics/src/routes/redirect-malicious/+page.js index 2f8cbe6b9caa..4755f05dc23a 100644 --- a/packages/kit/test/prerendering/basics/src/routes/redirect-malicious/+page.js +++ b/packages/kit/test/prerendering/basics/src/routes/redirect-malicious/+page.js @@ -2,5 +2,5 @@ import { redirect } from '@sveltejs/kit'; /** @type {import('@sveltejs/kit').Load} */ export function load() { - redirect(301, 'https://example.com/alert("pwned")'); + redirect(301, 'https://example.com/alert("pwned")', { external: true }); } diff --git a/packages/kit/test/prerendering/basics/src/routes/redirect-server/+page.server.js b/packages/kit/test/prerendering/basics/src/routes/redirect-server/+page.server.js index c61c65b8d644..fc04229e84a8 100644 --- a/packages/kit/test/prerendering/basics/src/routes/redirect-server/+page.server.js +++ b/packages/kit/test/prerendering/basics/src/routes/redirect-server/+page.server.js @@ -2,5 +2,5 @@ import { redirect } from '@sveltejs/kit'; /** @type {import('@sveltejs/kit').Load} */ export function load() { - redirect(301, 'https://example.com/redirected'); + redirect(301, 'https://example.com/redirected', { external: true }); } diff --git a/packages/kit/test/prerendering/basics/src/routes/redirect/+page.js b/packages/kit/test/prerendering/basics/src/routes/redirect/+page.js index c61c65b8d644..fc04229e84a8 100644 --- a/packages/kit/test/prerendering/basics/src/routes/redirect/+page.js +++ b/packages/kit/test/prerendering/basics/src/routes/redirect/+page.js @@ -2,5 +2,5 @@ import { redirect } from '@sveltejs/kit'; /** @type {import('@sveltejs/kit').Load} */ export function load() { - redirect(301, 'https://example.com/redirected'); + redirect(301, 'https://example.com/redirected', { external: true }); } diff --git a/packages/kit/test/prerendering/basics/src/routes/spa-shell/+page.js b/packages/kit/test/prerendering/basics/src/routes/spa-shell/+page.js index 55a527fff567..6714fee4a995 100644 --- a/packages/kit/test/prerendering/basics/src/routes/spa-shell/+page.js +++ b/packages/kit/test/prerendering/basics/src/routes/spa-shell/+page.js @@ -6,5 +6,5 @@ export const ssr = false; /** @type {import('@sveltejs/kit').Load} */ export function load() { - redirect(301, 'https://example.com/redirected'); + redirect(301, 'https://example.com/redirected', { external: true }); } diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 75775f8ffabb..70d193237e9c 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -2893,10 +2893,13 @@ declare module '@sveltejs/kit' { * * @param status The [HTTP status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#redirection_messages). Must be in the range 300-308. * @param location The location to redirect to. + * @param options To redirect to an external URL, you must pass `{ external: true }` to allow any external URL except `javascript:` URLs, or `{ external: [...] }` with an allowlist of permitted origins. * @throws {import('./public.js').Redirect} This error instructs SvelteKit to redirect to the specified location. - * @throws {Error} If the provided status is invalid or the location cannot be used as a header value. + * @throws {Error} If the provided status is invalid, the location cannot be used as a header value, or the location is an external URL without permission. * */ - export function redirect(status: 300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308 | ({} & number), location: string | URL): never; + export function redirect(status: 300 | 301 | 302 | 303 | 304 | 305 | 306 | 307 | 308 | ({} & number), location: string | URL, options?: { + external?: boolean | string[]; + }): never; /** * Checks whether this is a redirect thrown by {@link redirect}. * @param e The object to check.