diff --git a/.changeset/quick-eels-join.md b/.changeset/quick-eels-join.md new file mode 100644 index 0000000000..c9116d9304 --- /dev/null +++ b/.changeset/quick-eels-join.md @@ -0,0 +1,5 @@ +--- +"@react-router/node": patch +--- + +Validate format of incoming session ids diff --git a/packages/react-router-node/__tests__/sessions-test.ts b/packages/react-router-node/__tests__/sessions-test.ts index 9a72c5b5ec..95446861a5 100644 --- a/packages/react-router-node/__tests__/sessions-test.ts +++ b/packages/react-router-node/__tests__/sessions-test.ts @@ -55,6 +55,32 @@ describe("File session storage", () => { expect(session.get("user")).toBeUndefined(); }); + it("returns an empty session for invalid session ids", async () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let { getSession, commitSession } = createFileSessionStorage({ + dir, + }); + + let cookie = `__session=${btoa(JSON.stringify("0123456789abcdef"))}`; + let session = await getSession(cookie); + session.set("user", "mjackson"); + expect(session.get("user")).toBe("mjackson"); + let setCookie = await commitSession(session); + session = await getSession(getCookieFromSetCookie(setCookie)); + expect(session.get("user")).toBe("mjackson"); + + cookie = `__session=${btoa(JSON.stringify("0123456789abcdeg"))}`; + session = await getSession(cookie); + session.set("user", "mjackson"); + expect(session.get("user")).toBe("mjackson"); + debugger; + setCookie = await commitSession(session); + session = await getSession(getCookieFromSetCookie(setCookie)); + expect(session.get("user")).toBeUndefined(); + + spy.mockRestore(); + }); + it("doesn't destroy the entire session directory when destroying an empty file session", async () => { let { getSession, destroySession } = createFileSessionStorage({ dir, diff --git a/packages/react-router-node/sessions/fileStorage.ts b/packages/react-router-node/sessions/fileStorage.ts index 0df4bacb4d..3d2c30ae41 100644 --- a/packages/react-router-node/sessions/fileStorage.ts +++ b/packages/react-router-node/sessions/fileStorage.ts @@ -47,6 +47,9 @@ export function createFileSessionStorage({ try { let file = getFile(dir, id); + if (!file) { + throw new Error("Error generating session"); + } await fsp.mkdir(path.dirname(file), { recursive: true }); await fsp.writeFile(file, content, { encoding: "utf-8", flag: "wx" }); return id; @@ -58,6 +61,9 @@ export function createFileSessionStorage({ async readData(id) { try { let file = getFile(dir, id); + if (!file) { + return null; + } let content = JSON.parse(await fsp.readFile(file, "utf-8")); let data = content.data; let expires = @@ -81,6 +87,9 @@ export function createFileSessionStorage({ async updateData(id, data, expires) { let content = JSON.stringify({ data, expires }); let file = getFile(dir, id); + if (!file) { + return; + } await fsp.mkdir(path.dirname(file), { recursive: true }); await fsp.writeFile(file, content, "utf-8"); }, @@ -90,8 +99,12 @@ export function createFileSessionStorage({ if (!id) { return; } + let file = getFile(dir, id); + if (!file) { + return; + } try { - await fsp.unlink(getFile(dir, id)); + await fsp.unlink(file); } catch (error: any) { if (error.code !== "ENOENT") throw error; } @@ -99,7 +112,11 @@ export function createFileSessionStorage({ }); } -export function getFile(dir: string, id: string): string { +export function getFile(dir: string, id: string): string | null { + if (!/^[0-9a-f]{16}$/i.test(id)) { + return null; + } + // Divide the session id up into a directory (first 2 bytes) and filename // (remaining 6 bytes) to reduce the chance of having very large directories, // which should speed up file access. This is a maximum of 2^16 directories,