Add some validation for session IDs

Author: brophdawg11

.changeset/quick-eels-join.md

@@ -0,0 +1,5 @@
+---
+"@react-router/node": patch
+---
+
+Validate format of incoming session ids

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,

packages/react-router-node/sessions/fileStorage.ts

@@ -47,6 +47,9 @@ export function createFileSessionStorage<Data = SessionData, FlashData = Data>({
 
         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<Data = SessionData, FlashData = Data>({
     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<Data = SessionData, FlashData = Data>({
     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,16 +99,24 @@ export function createFileSessionStorage<Data = SessionData, FlashData = Data>({
       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;
       }
     },
   });
 }
 
-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,