Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/redirect-catch-all-hardening.md
Original file line number Diff line number Diff line change
@@ -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
10 changes: 6 additions & 4 deletions packages/astro/src/core/redirects/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/core/routing/generator.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -41,7 +42,7 @@ function getParameter(part: RoutePart, params: Record<string, string | number>):
function getSegment(segment: RoutePart[], params: Record<string, string | number>): string {
const segmentPath = segment.map((part) => getParameter(part, params)).join('');

return segmentPath ? '/' + segmentPath : '';
return segmentPath ? collapseDuplicateLeadingSlashes('/' + segmentPath) : '';
}

type RouteGenerator = (data?: any) => string;
Expand Down
102 changes: 102 additions & 0 deletions packages/astro/test/units/redirects/open-redirect.test.js
Original file line number Diff line number Diff line change
@@ -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}'`,
);
});
});
});