Fix absolute path traversal bypass with backslash conversion

I also found out we didn't check \t \r \n for absolute paths (if it's an
URL object)

Author: hansott

library/sinks/FileSystem.test.ts

@@ -277,11 +277,46 @@ t.test("it works", async (t) => {
     }
   );
 
+  // same as above but starts with dangerous path
+  runWithContext(
+    {
+      remoteAddress: "::1",
+      method: "POST",
+      url: "http://localhost:4000",
+      query: {
+        q: "/etc\t/passwd",
+      },
+      headers: {},
+      body: {},
+      cookies: {},
+      routeParams: {},
+      source: "express",
+      route: "/posts/:id",
+    },
+    () => {
+      throws(
+        () => rename(new URL("file:///etc\t/passwd")),
+        "Zen has blocked a path traversal attack: fs.rename(...) originating from query.q"
+      );
+    }
+  );
+
   // Ignores malformed URLs
   runWithContext(
     { ...unsafeContext, body: { file: { matches: "../%" } } },
     () => {
       rename(new URL("file:///../../test.txt"), "../test2.txt", () => {});
     }
   );
+
+  // new URL("file://\\etc/passwd") becomes "file:///etc/passwd"
+  runWithContext(
+    { ...unsafeContext, body: { file: { matches: "\\etc/passwd" } } },
+    () => {
+      throws(
+        () => rename(new URL("file://\\etc/passwd")),
+        "Zen has blocked a path traversal attack: fs.rename(...) originating from body.file.matches"
+      );
+    }
+  );
 });

library/sources/HTTPServer.test.ts

@@ -830,3 +830,89 @@ t.test("it blocks double encoded path traversal", async (t) => {
     });
   });
 });
+
+t.test("it blocks absolute path traversal with tab inside", async (t) => {
+  const server = http.createServer((req, res) => {
+    try {
+      const query = new URL(
+        req.url!.startsWith("http") ? req.url! : `http://acme.com${req.url!}`
+      ).searchParams;
+      const file = readFileSync(new URL(`file://${query.get("path")}`));
+
+      res.statusCode = 200;
+      res.end(file);
+    } catch (error) {
+      res.statusCode = 500;
+      if (error instanceof Error) {
+        res.end(error.message);
+        return;
+      }
+      res.end("Internal server error");
+    }
+  });
+
+  await new Promise<void>((resolve) => {
+    server.listen(3328, async () => {
+      fetch({
+        url: new URL(
+          `http://localhost:3328/?path=/etc${encodeURIComponent("\t")}/passwd`
+        ),
+        method: "GET",
+        headers: {
+          "x-forwarded-for": "1.2.3.4",
+        },
+        timeoutInMS: 500,
+      }).then(({ statusCode, body }) => {
+        t.equal(statusCode, 500);
+        t.equal(
+          body,
+          "Zen has blocked a path traversal attack: fs.readFileSync(...) originating from query.path"
+        );
+        server.close();
+        resolve();
+      });
+    });
+  });
+});
+
+t.test("it blocks backslash path traversal", async (t) => {
+  const server = http.createServer((req, res) => {
+    try {
+      const query = new URL(
+        req.url!.startsWith("http") ? req.url! : `http://acme.com${req.url!}`
+      ).searchParams;
+      const file = readFileSync(new URL(`file://${query.get("path")}`));
+
+      res.statusCode = 200;
+      res.end(file);
+    } catch (error) {
+      res.statusCode = 500;
+      if (error instanceof Error) {
+        res.end(error.message);
+        return;
+      }
+      res.end("Internal server error");
+    }
+  });
+
+  await new Promise<void>((resolve) => {
+    server.listen(3328, async () => {
+      fetch({
+        url: new URL("http://localhost:3328/?path=\\etc/passwd"),
+        method: "GET",
+        headers: {
+          "x-forwarded-for": "1.2.3.4",
+        },
+        timeoutInMS: 500,
+      }).then(({ statusCode, body }) => {
+        t.equal(statusCode, 500);
+        t.equal(
+          body,
+          "Zen has blocked a path traversal attack: fs.readFileSync(...) originating from query.path"
+        );
+        server.close();
+        resolve();
+      });
+    });
+  });
+});

library/vulnerabilities/path-traversal/containsUnsafePathParts.ts

@@ -1,3 +1,5 @@
+import { normalizeLikeURLConstructor } from "./normalizeLikeURLConstructor";
+
 const dangerousPathParts = ["../", "..\\"];
 
 export function containsUnsafePathParts(filePath: string) {
@@ -10,16 +12,6 @@ export function containsUnsafePathParts(filePath: string) {
   return false;
 }
 
-/**
- * This function is used for urls, because they can contain a TAB, carriage return or line feed that is silently removed by the URL constructor.
- *
- * The WHATWG URL spec defines the following:
- * - Remove all ASCII tab or newline from input.
- * - An ASCII tab or newline is U+0009 TAB, U+000A LF, or U+000D CR.
- *
- * See https://url.spec.whatwg.org/#url-parsing
- */
 export function containsUnsafePathPartsUrl(filePath: string) {
-  const normalized = filePath.replace(/[\t\n\r]/g, "");
-  return containsUnsafePathParts(normalized);
+  return containsUnsafePathParts(normalizeLikeURLConstructor(filePath));
 }

library/vulnerabilities/path-traversal/detectPathTraversal.ts

@@ -2,7 +2,10 @@ import {
   containsUnsafePathParts,
   containsUnsafePathPartsUrl,
 } from "./containsUnsafePathParts";
-import { startsWithUnsafePath } from "./unsafePathStart";
+import {
+  startsWithUnsafePath,
+  startsWithUnsafePathUrl,
+} from "./unsafePathStart";
 import { fileURLToPath } from "url";
 
 export function detectPathTraversal(
@@ -19,13 +22,18 @@ export function detectPathTraversal(
   // Check for URL path traversal
   // Reason: new URL("file:///../../test.txt") => /test.txt
   // The normal check for relative path traversal will fail in this case, because transformed path does not contain ../.
-  // For absolute path traversal, we dont need to check the transformed path, because it will always start with /.
   // Also /./ is checked by normal absolute path traversal check (if #219 is merged)
   // Use containsUnsafePathPartsUrl, because urls can contain a TAB, carriage return or line feed that is silently removed by the URL constructor.
-  if (isUrl && containsUnsafePathPartsUrl(userInput)) {
-    const filePathFromUrl = parseAsFileUrl(userInput);
-    if (filePathFromUrl && filePath.includes(filePathFromUrl)) {
-      return true;
+  // Use startsWithUnsafePathUrl, because URLs can contain backward slashes that are converted to forward slashes by the URL constructor.
+  if (isUrl) {
+    const containsUnsafePath = containsUnsafePathPartsUrl(userInput);
+    const startWithUnsafePath = startsWithUnsafePathUrl(filePath, userInput);
+
+    if (containsUnsafePath || startWithUnsafePath) {
+      const filePathFromUrl = parseAsFileUrl(userInput);
+      if (filePathFromUrl && filePath.includes(filePathFromUrl)) {
+        return true;
+      }
     }
   }
 
@@ -63,7 +71,7 @@ export function detectPathTraversal(
 function parseAsFileUrl(path: string) {
   let url = path;
   if (!url.startsWith("file:")) {
-    if (!url.startsWith("/")) {
+    if (!url.startsWith("/") && !url.startsWith("\\")) {
       url = `/${url}`;
     }
     url = `file://${url}`;

library/vulnerabilities/path-traversal/normalizeLikeURLConstructor.ts

@@ -0,0 +1,14 @@
+/**
+ * This function is used for urls, because they can contain a TAB, carriage return or line feed that is silently removed by the URL constructor.
+ *
+ * The WHATWG URL spec defines the following:
+ * - Remove all ASCII tab or newline from input.
+ * - An ASCII tab or newline is U+0009 TAB, U+000A LF, or U+000D CR.
+ *
+ * Also, backward slashes are converted to forward slashes by the URL constructor.
+ *
+ * See https://url.spec.whatwg.org/#url-parsing
+ */
+export function normalizeLikeURLConstructor(url: string): string {
+  return url.replace(/[\t\n\r]/g, "").replace(/\\/g, "/");
+}

library/vulnerabilities/path-traversal/unsafePathStart.ts

@@ -1,5 +1,6 @@
 import { isAbsolute, resolve } from "path";
 import { isWrapped } from "../../helpers/wrap";
+import { normalizeLikeURLConstructor } from "./normalizeLikeURLConstructor";
 
 const linuxRootFolders = [
   "/bin/",
@@ -57,3 +58,7 @@ export function startsWithUnsafePath(filePath: string, userInput: string) {
   }
   return false;
 }
+
+export function startsWithUnsafePathUrl(filePath: string, userInput: string) {
+  return startsWithUnsafePath(filePath, normalizeLikeURLConstructor(userInput));
+}