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/polite-balloons-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

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
5 changes: 5 additions & 0 deletions packages/astro/src/core/app/base.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
appendForwardSlash,
collapseDuplicateLeadingSlashes,
collapseDuplicateTrailingSlashes,
hasFileExtension,
isInternalPath,
Expand Down Expand Up @@ -187,6 +188,10 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
}

public removeBase(pathname: string) {
// Collapse multiple leading slashes to prevent middleware authorization bypass.
// Without this, `//admin` would be treated as starting with base `/` and sliced
// to `/admin` for routing, while middleware still sees `//admin` in the URL.
pathname = collapseDuplicateLeadingSlashes(pathname);
if (pathname.startsWith(this.manifest.base)) {
return pathname.slice(this.baseWithoutTrailingSlash.length + 1);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { getParams, getProps, type Pipeline, Slots } from './render/index.js';
import { isRoute404or500, isRouteExternalRedirect, isRouteServerIsland } from './routing/match.js';
import { copyRequest, getOriginPathname, setOriginPathname } from './routing/rewrite.js';
import { AstroSession } from './session/runtime.js';
import { collapseDuplicateLeadingSlashes } from '@astrojs/internal-helpers/path';
import { validateAndDecodePathname } from './util/pathname.js';

/**
Expand Down Expand Up @@ -86,6 +87,10 @@ export class RenderContext {

static #createNormalizedUrl(requestUrl: string): URL {
const url = new URL(requestUrl);
// Collapse multiple leading slashes so middleware sees the canonical pathname.
// Without this, a request to `//admin` would preserve `//admin` in context.url.pathname,
// bypassing middleware checks like `pathname.startsWith('/admin')`.
url.pathname = collapseDuplicateLeadingSlashes(url.pathname);
try {
// Decode and validate pathname to prevent multi-level encoding bypass attacks
url.pathname = validateAndDecodePathname(url.pathname);
Expand Down
162 changes: 162 additions & 0 deletions packages/astro/test/units/app/double-slash-bypass.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
// @ts-check
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { App } from '../../../dist/core/app/app.js';
import { parseRoute } from '../../../dist/core/routing/parse-route.js';
import { createComponent, render } from '../../../dist/runtime/server/index.js';
import { createManifest } from './test-helpers.js';

/**
* Security tests for double-slash URL prefix middleware authorization bypass.
*
* Vulnerability: A normalization inconsistency between route matching and middleware
* URL construction allows bypassing middleware-based authorization by prepending an
* extra `/` to the URL path (e.g., `//admin` instead of `/admin`).
*
* - `removeBase("//admin")` strips one slash → router matches `/admin`
* - `context.url.pathname` preserves `//admin` → middleware `startsWith("/admin")` fails
*
* See: withastro/astro-security#5
* CWE-647: Use of Non-Canonical URL Paths for Authorization Decisions
* CWE-285: Improper Authorization
*/

const routeOptions = /** @type {Parameters<typeof parseRoute>[1]} */ (
/** @type {any} */ ({
config: { base: '/', trailingSlash: 'ignore' },
pageExtensions: [],
})
);

const adminRouteData = parseRoute('admin', routeOptions, {
component: 'src/pages/admin.astro',
});

const dashboardRouteData = parseRoute('dashboard', routeOptions, {
component: 'src/pages/dashboard.astro',
});

const publicRouteData = parseRoute('index.astro', routeOptions, {
component: 'src/pages/index.astro',
});

const adminPage = createComponent(() => {
return render`<h1>Admin Panel</h1>`;
});

const dashboardPage = createComponent(() => {
return render`<h1>Dashboard</h1>`;
});

const publicPage = createComponent(() => {
return render`<h1>Public</h1>`;
});

const pageMap = new Map([
[
adminRouteData.component,
async () => ({
page: async () => ({
default: adminPage,
}),
}),
],
[
dashboardRouteData.component,
async () => ({
page: async () => ({
default: dashboardPage,
}),
}),
],
[
publicRouteData.component,
async () => ({
page: async () => ({
default: publicPage,
}),
}),
],
]);

/**
* Middleware that blocks access to /admin and /dashboard routes,
* as recommended in the official Astro authentication docs.
* @returns {() => Promise<{onRequest: import('../../../dist/types/public/common.js').MiddlewareHandler}>}
*/
function createAuthMiddleware() {
return async () => ({
onRequest: /** @type {import('../../../dist/types/public/common.js').MiddlewareHandler} */ (
async (context, next) => {
const protectedPaths = ['/admin', '/dashboard'];
if (protectedPaths.some((p) => context.url.pathname.startsWith(p))) {
return new Response('Forbidden', { status: 403 });
}
return next();
}
),
});
}

/**
* @param {ReturnType<typeof createAuthMiddleware>} middleware
*/
function createApp(middleware) {
return new App(
createManifest({
routes: [
{ routeData: adminRouteData },
{ routeData: dashboardRouteData },
{ routeData: publicRouteData },
],
pageMap,
middleware,
}),
);
}

describe('Security: double-slash URL prefix middleware bypass', () => {
it('middleware blocks /admin with normal request', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/admin');
const response = await app.render(request);
assert.equal(response.status, 403, '/admin should be blocked by middleware');
});

it('middleware blocks //admin (double-slash bypass attempt)', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com//admin');
const response = await app.render(request);
assert.equal(response.status, 403, '//admin should also be blocked by middleware');
});

it('middleware blocks ///admin (triple-slash bypass attempt)', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com///admin');
const response = await app.render(request);
assert.equal(response.status, 403, '///admin should also be blocked by middleware');
});

it('middleware blocks //dashboard (double-slash on another protected route)', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com//dashboard');
const response = await app.render(request);
assert.equal(response.status, 403, '//dashboard should also be blocked by middleware');
});

it('middleware blocks //admin/ (double-slash with trailing slash)', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com//admin/');
const response = await app.render(request);
assert.equal(response.status, 403, '//admin/ should also be blocked by middleware');
});

it('public route is still accessible', async () => {
const app = createApp(createAuthMiddleware());
const request = new Request('http://example.com/');
const response = await app.render(request);
assert.equal(response.status, 200, '/ should be accessible');
const html = await response.text();
assert.match(html, /Public/);
});
});
24 changes: 20 additions & 4 deletions packages/astro/test/units/app/test-helpers.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,24 @@
// @ts-check

export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'ignore' } = {}) {
/**
* @param {object} [options]
* @param {any[]} [options.routes]
* @param {Map<string, Function>} [options.pageMap]
* @param {string} [options.base]
* @param {string} [options.trailingSlash]
* @param {Function} [options.middleware]
*/
export function createManifest({
routes,
pageMap,
base = '/',
trailingSlash = 'ignore',
middleware = undefined,
} = {}) {
const rootDir = new URL('file:///astro-test/');
const buildDir = new URL('file:///astro-test/dist/');

return {
return /** @type {import('../../../dist/core/app/types.js').SSRManifest} */ ({
adapterName: 'test-adapter',
routes,
site: undefined,
Expand All @@ -16,6 +30,7 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
assetsPrefix: undefined,
renderers: [],
serverLike: true,
middlewareMode: /** @type {'classic'} */ ('classic'),
clientDirectives: new Map(),
entryModules: {},
inlinedScripts: new Map(),
Expand All @@ -26,11 +41,12 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
serverIslandMappings: undefined,
key: Promise.resolve(/** @type {CryptoKey} */ ({})),
i18n: undefined,
middleware: undefined,
middleware,
actions: undefined,
sessionDriver: undefined,
checkOrigin: false,
allowedDomains: undefined,
actionBodySizeLimit: 0,
sessionConfig: undefined,
cacheDir: rootDir,
srcDir: rootDir,
Expand All @@ -54,7 +70,7 @@ export function createManifest({ routes, pageMap, base = '/', trailingSlash = 'i
experimentalQueuedRendering: {
enabled: false,
},
};
});
}

export function createRouteInfo(routeData) {
Expand Down
9 changes: 9 additions & 0 deletions packages/internal-helpers/src/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ export function prependForwardSlash(path: string) {
return path[0] === '/' ? path : '/' + path;
}

export const MANY_LEADING_SLASHES = /^\/{2,}/;

export function collapseDuplicateLeadingSlashes(path: string) {
if (!path) {
return path;
}
return path.replace(MANY_LEADING_SLASHES, '/');
}

export const MANY_TRAILING_SLASHES = /\/{2,}$/g;

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