Skip to content

Commit 4000aaa

Browse files
authored
Harden URL pathname normalization to collapse multiple leading slashes (#15717)
* Harden URL pathname normalization to collapse multiple leading slashes * Refactor: use collapseDuplicateLeadingSlashes utility from internal-helpers * Update changeset to be more user-facing
1 parent 4b54eb5 commit 4000aaa

6 files changed

Lines changed: 206 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Ensures that URLs with multiple leading slashes (e.g. `//admin`) are normalized to a single slash before reaching middleware, so that pathname checks like `context.url.pathname.startsWith('/admin')` work consistently regardless of the request URL format

packages/astro/src/core/app/base.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
appendForwardSlash,
3+
collapseDuplicateLeadingSlashes,
34
collapseDuplicateTrailingSlashes,
45
hasFileExtension,
56
isInternalPath,
@@ -195,6 +196,10 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
195196
}
196197

197198
public removeBase(pathname: string) {
199+
// Collapse multiple leading slashes to prevent middleware authorization bypass.
200+
// Without this, `//admin` would be treated as starting with base `/` and sliced
201+
// to `/admin` for routing, while middleware still sees `//admin` in the URL.
202+
pathname = collapseDuplicateLeadingSlashes(pathname);
198203
if (pathname.startsWith(this.manifest.base)) {
199204
return pathname.slice(this.baseWithoutTrailingSlash.length + 1);
200205
}

packages/astro/src/core/render-context.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { getParams, getProps, type Pipeline, Slots } from './render/index.js';
3737
import { isRoute404or500, isRouteExternalRedirect, isRouteServerIsland } from './routing/match.js';
3838
import { copyRequest, getOriginPathname, setOriginPathname } from './routing/rewrite.js';
3939
import { AstroSession } from './session/runtime.js';
40+
import { collapseDuplicateLeadingSlashes } from '@astrojs/internal-helpers/path';
4041
import { validateAndDecodePathname } from './util/pathname.js';
4142

4243
/**
@@ -86,6 +87,10 @@ export class RenderContext {
8687

8788
static #createNormalizedUrl(requestUrl: string): URL {
8889
const url = new URL(requestUrl);
90+
// Collapse multiple leading slashes so middleware sees the canonical pathname.
91+
// Without this, a request to `//admin` would preserve `//admin` in context.url.pathname,
92+
// bypassing middleware checks like `pathname.startsWith('/admin')`.
93+
url.pathname = collapseDuplicateLeadingSlashes(url.pathname);
8994
try {
9095
// Decode and validate pathname to prevent multi-level encoding bypass attacks
9196
url.pathname = validateAndDecodePathname(url.pathname);
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
// @ts-check
2+
import assert from 'node:assert/strict';
3+
import { describe, it } from 'node:test';
4+
import { App } from '../../../dist/core/app/app.js';
5+
import { parseRoute } from '../../../dist/core/routing/parse-route.js';
6+
import { createComponent, render } from '../../../dist/runtime/server/index.js';
7+
import { createManifest } from './test-helpers.js';
8+
9+
/**
10+
* Security tests for double-slash URL prefix middleware authorization bypass.
11+
*
12+
* Vulnerability: A normalization inconsistency between route matching and middleware
13+
* URL construction allows bypassing middleware-based authorization by prepending an
14+
* extra `/` to the URL path (e.g., `//admin` instead of `/admin`).
15+
*
16+
* - `removeBase("//admin")` strips one slash → router matches `/admin`
17+
* - `context.url.pathname` preserves `//admin` → middleware `startsWith("/admin")` fails
18+
*
19+
* See: withastro/astro-security#5
20+
* CWE-647: Use of Non-Canonical URL Paths for Authorization Decisions
21+
* CWE-285: Improper Authorization
22+
*/
23+
24+
const routeOptions = /** @type {Parameters<typeof parseRoute>[1]} */ (
25+
/** @type {any} */ ({
26+
config: { base: '/', trailingSlash: 'ignore' },
27+
pageExtensions: [],
28+
})
29+
);
30+
31+
const adminRouteData = parseRoute('admin', routeOptions, {
32+
component: 'src/pages/admin.astro',
33+
});
34+
35+
const dashboardRouteData = parseRoute('dashboard', routeOptions, {
36+
component: 'src/pages/dashboard.astro',
37+
});
38+
39+
const publicRouteData = parseRoute('index.astro', routeOptions, {
40+
component: 'src/pages/index.astro',
41+
});
42+
43+
const adminPage = createComponent(() => {
44+
return render`<h1>Admin Panel</h1>`;
45+
});
46+
47+
const dashboardPage = createComponent(() => {
48+
return render`<h1>Dashboard</h1>`;
49+
});
50+
51+
const publicPage = createComponent(() => {
52+
return render`<h1>Public</h1>`;
53+
});
54+
55+
const pageMap = new Map([
56+
[
57+
adminRouteData.component,
58+
async () => ({
59+
page: async () => ({
60+
default: adminPage,
61+
}),
62+
}),
63+
],
64+
[
65+
dashboardRouteData.component,
66+
async () => ({
67+
page: async () => ({
68+
default: dashboardPage,
69+
}),
70+
}),
71+
],
72+
[
73+
publicRouteData.component,
74+
async () => ({
75+
page: async () => ({
76+
default: publicPage,
77+
}),
78+
}),
79+
],
80+
]);
81+
82+
/**
83+
* Middleware that blocks access to /admin and /dashboard routes,
84+
* as recommended in the official Astro authentication docs.
85+
* @returns {() => Promise<{onRequest: import('../../../dist/types/public/common.js').MiddlewareHandler}>}
86+
*/
87+
function createAuthMiddleware() {
88+
return async () => ({
89+
onRequest: /** @type {import('../../../dist/types/public/common.js').MiddlewareHandler} */ (
90+
async (context, next) => {
91+
const protectedPaths = ['/admin', '/dashboard'];
92+
if (protectedPaths.some((p) => context.url.pathname.startsWith(p))) {
93+
return new Response('Forbidden', { status: 403 });
94+
}
95+
return next();
96+
}
97+
),
98+
});
99+
}
100+
101+
/**
102+
* @param {ReturnType<typeof createAuthMiddleware>} middleware
103+
*/
104+
function createApp(middleware) {
105+
return new App(
106+
createManifest({
107+
routes: [
108+
{ routeData: adminRouteData },
109+
{ routeData: dashboardRouteData },
110+
{ routeData: publicRouteData },
111+
],
112+
pageMap,
113+
middleware,
114+
}),
115+
);
116+
}
117+
118+
describe('Security: double-slash URL prefix middleware bypass', () => {
119+
it('middleware blocks /admin with normal request', async () => {
120+
const app = createApp(createAuthMiddleware());
121+
const request = new Request('http://example.com/admin');
122+
const response = await app.render(request);
123+
assert.equal(response.status, 403, '/admin should be blocked by middleware');
124+
});
125+
126+
it('middleware blocks //admin (double-slash bypass attempt)', async () => {
127+
const app = createApp(createAuthMiddleware());
128+
const request = new Request('http://example.com//admin');
129+
const response = await app.render(request);
130+
assert.equal(response.status, 403, '//admin should also be blocked by middleware');
131+
});
132+
133+
it('middleware blocks ///admin (triple-slash bypass attempt)', async () => {
134+
const app = createApp(createAuthMiddleware());
135+
const request = new Request('http://example.com///admin');
136+
const response = await app.render(request);
137+
assert.equal(response.status, 403, '///admin should also be blocked by middleware');
138+
});
139+
140+
it('middleware blocks //dashboard (double-slash on another protected route)', async () => {
141+
const app = createApp(createAuthMiddleware());
142+
const request = new Request('http://example.com//dashboard');
143+
const response = await app.render(request);
144+
assert.equal(response.status, 403, '//dashboard should also be blocked by middleware');
145+
});
146+
147+
it('middleware blocks //admin/ (double-slash with trailing slash)', async () => {
148+
const app = createApp(createAuthMiddleware());
149+
const request = new Request('http://example.com//admin/');
150+
const response = await app.render(request);
151+
assert.equal(response.status, 403, '//admin/ should also be blocked by middleware');
152+
});
153+
154+
it('public route is still accessible', async () => {
155+
const app = createApp(createAuthMiddleware());
156+
const request = new Request('http://example.com/');
157+
const response = await app.render(request);
158+
assert.equal(response.status, 200, '/ should be accessible');
159+
const html = await response.text();
160+
assert.match(html, /Public/);
161+
});
162+
});

packages/astro/test/units/app/test-helpers.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,24 @@
11
// @ts-check
22

3-
export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'ignore' } = {}) {
3+
/**
4+
* @param {object} [options]
5+
* @param {any[]} [options.routes]
6+
* @param {Map<string, Function>} [options.pageMap]
7+
* @param {string} [options.base]
8+
* @param {string} [options.trailingSlash]
9+
* @param {Function} [options.middleware]
10+
*/
11+
export function createManifest({
12+
routes,
13+
pageMap,
14+
base = '/',
15+
trailingSlash = 'ignore',
16+
middleware = undefined,
17+
} = {}) {
418
const rootDir = new URL('file:///astro-test/');
519
const buildDir = new URL('file:///astro-test/dist/');
620

7-
return {
21+
return /** @type {import('../../../dist/core/app/types.js').SSRManifest} */ ({
822
adapterName: 'test-adapter',
923
routes,
1024
site: undefined,
@@ -16,6 +30,7 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
1630
assetsPrefix: undefined,
1731
renderers: [],
1832
serverLike: true,
33+
middlewareMode: /** @type {'classic'} */ ('classic'),
1934
clientDirectives: new Map(),
2035
entryModules: {},
2136
inlinedScripts: new Map(),
@@ -26,11 +41,12 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
2641
serverIslandMappings: undefined,
2742
key: Promise.resolve(/** @type {CryptoKey} */ ({})),
2843
i18n: undefined,
29-
middleware: undefined,
44+
middleware,
3045
actions: undefined,
3146
sessionDriver: undefined,
3247
checkOrigin: false,
3348
allowedDomains: undefined,
49+
actionBodySizeLimit: 0,
3450
sessionConfig: undefined,
3551
cacheDir: rootDir,
3652
srcDir: rootDir,
@@ -54,7 +70,7 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
5470
experimentalQueuedRendering: {
5571
enabled: false,
5672
},
57-
};
73+
});
5874
}
5975

6076
export function createRouteInfo(routeData) {

packages/internal-helpers/src/path.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ export function prependForwardSlash(path: string) {
1515
return path[0] === '/' ? path : '/' + path;
1616
}
1717

18+
export const MANY_LEADING_SLASHES = /^\/{2,}/;
19+
20+
export function collapseDuplicateLeadingSlashes(path: string) {
21+
if (!path) {
22+
return path;
23+
}
24+
return path.replace(MANY_LEADING_SLASHES, '/');
25+
}
26+
1827
export const MANY_TRAILING_SLASHES = /\/{2,}$/g;
1928

2029
export function collapseDuplicateTrailingSlashes(path: string, trailingSlash: boolean) {

0 commit comments

Comments
 (0)