diff --git a/.changeset/redirect-catch-all-hardening.md b/.changeset/redirect-catch-all-hardening.md new file mode 100644 index 000000000000..4566f24c0d61 --- /dev/null +++ b/.changeset/redirect-catch-all-hardening.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hardens config-based redirects with catch-all parameters to prevent producing protocol-relative URLs (e.g. `//example.com`) in the `Location` header diff --git a/packages/astro/src/core/redirects/render.ts b/packages/astro/src/core/redirects/render.ts index eba4395b36ca..00a4543c109b 100644 --- a/packages/astro/src/core/redirects/render.ts +++ b/packages/astro/src/core/redirects/render.ts @@ -2,13 +2,15 @@ import type { RedirectConfig } from '../../types/public/index.js'; import type { RenderContext } from '../render-context.js'; import { getRouteGenerator } from '../routing/generator.js'; +function isExternalURL(url: string): boolean { + return url.startsWith('http://') || url.startsWith('https://') || url.startsWith('//'); +} + export function redirectIsExternal(redirect: RedirectConfig): boolean { if (typeof redirect === 'string') { - return redirect.startsWith('http://') || redirect.startsWith('https://'); + return isExternalURL(redirect); } else { - return ( - redirect.destination.startsWith('http://') || redirect.destination.startsWith('https://') - ); + return isExternalURL(redirect.destination); } } diff --git a/packages/astro/src/core/routing/generator.ts b/packages/astro/src/core/routing/generator.ts index 94dcf4324465..7ee9d8806aac 100644 --- a/packages/astro/src/core/routing/generator.ts +++ b/packages/astro/src/core/routing/generator.ts @@ -1,3 +1,4 @@ +import { collapseDuplicateLeadingSlashes } from '@astrojs/internal-helpers/path'; import type { AstroConfig } from '../../types/public/config.js'; import type { RoutePart } from '../../types/public/internal.js'; @@ -41,7 +42,7 @@ function getParameter(part: RoutePart, params: Record): function getSegment(segment: RoutePart[], params: Record): string { const segmentPath = segment.map((part) => getParameter(part, params)).join(''); - return segmentPath ? '/' + segmentPath : ''; + return segmentPath ? collapseDuplicateLeadingSlashes('/' + segmentPath) : ''; } type RouteGenerator = (data?: any) => string; diff --git a/packages/astro/test/units/redirects/open-redirect.test.js b/packages/astro/test/units/redirects/open-redirect.test.js new file mode 100644 index 000000000000..61f27b612014 --- /dev/null +++ b/packages/astro/test/units/redirects/open-redirect.test.js @@ -0,0 +1,102 @@ +// @ts-check +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { redirectIsExternal } from '../../../dist/core/redirects/render.js'; +import { getRouteGenerator } from '../../../dist/core/routing/generator.js'; + +describe('Protocol-relative URLs in redirects', () => { + describe('redirectIsExternal', () => { + it('detects http:// as external', () => { + assert.equal(redirectIsExternal('http://evil.com'), true); + }); + + it('detects https:// as external', () => { + assert.equal(redirectIsExternal('https://evil.com'), true); + }); + + it('detects protocol-relative //evil.com as external', () => { + assert.equal(redirectIsExternal('//evil.com'), true); + }); + + it('detects protocol-relative //evil.com/path as external', () => { + assert.equal(redirectIsExternal('//evil.com/path'), true); + }); + + it('does not flag normal paths as external', () => { + assert.equal(redirectIsExternal('/about'), false); + }); + + it('does not flag root path as external', () => { + assert.equal(redirectIsExternal('/'), false); + }); + + it('detects protocol-relative URL in object form', () => { + assert.equal(redirectIsExternal({ destination: '//evil.com', status: 301 }), true); + }); + }); + + describe('getRouteGenerator with root-level catch-all', () => { + it('does not produce protocol-relative URL when catch-all param contains leading slash', () => { + // Simulates destination '/[...slug]' — a single root-level catch-all segment + const segments = [[{ spread: true, content: '...slug', dynamic: true }]]; + const generator = getRouteGenerator(segments, 'never'); + + // When the request is '/old//evil.com/', the catch-all captures '/evil.com' + const result = generator({ slug: '/evil.com' }); + + // The result must NOT be '//evil.com' (protocol-relative URL) + assert.ok( + !result.startsWith('//'), + `Expected result to not start with '//', got '${result}'`, + ); + }); + + it('does not produce protocol-relative URL with trailing slash config', () => { + const segments = [[{ spread: true, content: '...slug', dynamic: true }]]; + const generator = getRouteGenerator(segments, 'always'); + + const result = generator({ slug: '/evil.com' }); + + assert.ok( + !result.startsWith('//'), + `Expected result to not start with '//', got '${result}'`, + ); + }); + + it('does not produce protocol-relative URL with subpath', () => { + const segments = [[{ spread: true, content: '...slug', dynamic: true }]]; + const generator = getRouteGenerator(segments, 'never'); + + const result = generator({ slug: '/evil.com/phish' }); + + assert.ok( + !result.startsWith('//'), + `Expected result to not start with '//', got '${result}'`, + ); + }); + + it('still produces correct paths for normal params', () => { + const segments = [[{ spread: true, content: '...slug', dynamic: true }]]; + const generator = getRouteGenerator(segments, 'never'); + + assert.equal(generator({ slug: 'about' }), '/about'); + assert.equal(generator({ slug: 'docs/getting-started' }), '/docs/getting-started'); + }); + + it('non-root catch-all is not affected', () => { + // Simulates destination '/new/[...slug]' — catch-all under a prefix + const segments = [ + [{ spread: false, content: 'new', dynamic: false }], + [{ spread: true, content: '...slug', dynamic: true }], + ]; + const generator = getRouteGenerator(segments, 'never'); + + // Even with a leading-slash param, the prefix prevents protocol-relative + const result = generator({ slug: '/evil.com' }); + assert.ok( + !result.startsWith('//'), + `Expected result to not start with '//', got '${result}'`, + ); + }); + }); +});