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
7 changes: 7 additions & 0 deletions .changeset/warm-mirrors-allow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

Reject cross-drive module paths in Pages Functions routing

On Windows, module paths using a different drive letter could be parsed in a way that bypassed the project-root check. These paths are now parsed correctly and rejected when they resolve outside the project.
53 changes: 53 additions & 0 deletions packages/wrangler/src/__tests__/pages/routes-module.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { UserError } from "@cloudflare/workers-utils";
import { describe, it } from "vitest";
import { writeRoutesModule } from "../../pages/functions/routes";
import { toUrlPath } from "../../paths";
import { runInTempDir } from "../helpers/run-in-tmp";

describe("routes module", () => {
runInTempDir();

it("accepts module paths when srcDir is a relative path", async ({
expect,
}) => {
await expect(
writeRoutesModule({
config: {
routes: [
{
routePath: toUrlPath("/"),
mountPath: toUrlPath("/"),
module: "hello.js:onRequest",
},
],
},
srcDir: "functions",
outfile: "_routes.js",
})
).resolves.toBeDefined();
});

it.skipIf(process.platform !== "win32")(
"rejects module paths on a different drive",
async ({ expect }) => {
const modulePath = String.raw`D:\evil.js`;
const config = {
routes: [
{
routePath: toUrlPath("/"),
mountPath: toUrlPath("/"),
module: modulePath,
},
],
};

await expect(
writeRoutesModule({
config,
srcDir: String.raw`C:\project`,
outfile: "_routes.js",
})
).rejects.toThrow(new UserError(`Invalid module path "${modulePath}"`));
}
);
});
28 changes: 21 additions & 7 deletions packages/wrangler/src/pages/functions/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export async function writeRoutesModule({
}

function parseConfig(config: Config, baseDir: string) {
baseDir = path.resolve(baseDir);
const routes: RoutesCollection = [];
const importMap: ImportMap = new Map();
const identifierCount = new Map<string, number>(); // to keep track of identifier collisions
Expand All @@ -77,23 +78,32 @@ function parseConfig(config: Config, baseDir: string) {
}

return paths.map((modulePath) => {
const [filepath, name = "default"] = modulePath.split(":");
let { identifier } = importMap.get(modulePath) ?? {};
const resolvedPath = path.resolve(baseDir, modulePath);
const moduleRoot = path.parse(resolvedPath).root;

const resolvedPath = path.resolve(baseDir, filepath);
// Strip the drive letter (if any) to avoid confusing the drive colon with the export name separator
const strippedPath = resolvedPath.slice(moduleRoot.length - 1);
const [filepath, name = "default"] = strippedPath.split(":");

const fullFilepath = path.resolve(moduleRoot, filepath);
const relativePath = path.relative(baseDir, fullFilepath);

// ensure the filepath isn't attempting to resolve to anything outside of the project
if (path.relative(baseDir, resolvedPath).startsWith("..")) {
throw new UserError(`Invalid module path "${filepath}"`);
if (
moduleRoot !== path.parse(baseDir).root ||
relativePath.startsWith("..")
) {
throw new UserError(`Invalid module path "${fullFilepath}"`);
}

// ensure the module name (if provided) is a valid identifier to guard against injection attacks
if (name !== "default" && !isValidIdentifier(name)) {
throw new UserError(`Invalid module identifier "${name}"`);
}

let { identifier } = importMap.get(resolvedPath) ?? {};
if (!identifier) {
identifier = normalizeIdentifier(`__${filepath}_${name}`);
identifier = normalizeIdentifier(`__${relativePath}_${name}`);

let count = identifierCount.get(identifier) ?? 0;
identifierCount.set(identifier, ++count);
Expand All @@ -102,7 +112,11 @@ function parseConfig(config: Config, baseDir: string) {
identifier += `_${count}`;
}

importMap.set(modulePath, { filepath: resolvedPath, name, identifier });
importMap.set(resolvedPath, {
filepath: fullFilepath,
name,
identifier,
});
}

return identifier;
Expand Down
Loading