Add backtrack protection to 6.x

pillarjs · Sep 12, 2024 · b891ab9 · b891ab9
1 parent 28a5b27
commit b891ab9
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 54 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -36,6 +36,7 @@
     "@types/node": "^20.4.9",
     "@types/semver": "^7.3.1",
     "@vitest/coverage-v8": "^1.4.0",
+    "recheck": "^4.4.5",
     "semver": "^7.3.5",
     "size-limit": "^11.1.2",
     "typescript": "^5.1.6"

diff --git a/redos.ts b/redos.ts
@@ -0,0 +1,21 @@
+import { checkSync } from "recheck";
+import { pathToRegexp } from "./src/index.js";
+
+let safe = 0;
+let fail = 0;
+
+const tests = ["/:x{/foobar/:y}?-:z"];
+
+for (const path of tests) {
+  const regexp = pathToRegexp(path);
+  const result = checkSync(regexp.source, regexp.flags);
+  if (result.status === "safe") {
+    safe++;
+    console.log("Safe:", path, String(regexp));
+  } else {
+    fail++;
+    console.log("Fail:", path, String(regexp));
+  }
+}
+
+console.log("Safe:", safe, "Fail:", fail);
diff --git a/src/index.spec.ts b/src/index.spec.ts
@@ -1353,7 +1353,7 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "+",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
     ],
     [
@@ -1397,7 +1397,7 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
       ".",
     ],
@@ -1430,13 +1430,13 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
     ],
     [
       ["/route.html", ["/route.html", "route", "html"]],
       ["/route", null],
-      ["/route.html.json", ["/route.html.json", "route", "html.json"]],
+      ["/route.html.json", ["/route.html.json", "route.html", "json"]],
     ],
     [
       [{}, null],
@@ -1459,13 +1459,13 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "?",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
     ],
     [
       ["/route", ["/route", "route", undefined]],
       ["/route.json", ["/route.json", "route", "json"]],
-      ["/route.json.html", ["/route.json.html", "route", "json.html"]],
+      ["/route.json.html", ["/route.json.html", "route.json", "html"]],
     ],
     [
       [{ test: "route" }, "/route"],
@@ -1491,13 +1491,13 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "?",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
     ],
     [
       ["/route", ["/route", "route", undefined]],
       ["/route.json", ["/route.json", "route", "json"]],
-      ["/route.json.html", ["/route.json.html", "route", "json.html"]],
+      ["/route.json.html", ["/route.json.html", "route.json", "html"]],
     ],
     [
       [{ test: "route" }, "/route"],
@@ -2084,7 +2084,7 @@ const TESTS: Test[] = [
         prefix: "",
         suffix: "",
         modifier: "?",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\()[^\\/#\\?])+?",
       },
       ")",
     ],
@@ -2290,7 +2290,7 @@ const TESTS: Test[] = [
         prefix: ".",
         suffix: "",
         modifier: "",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\.)[^\\/#\\?])+?",
       },
     ],
     [
@@ -2356,14 +2356,14 @@ const TESTS: Test[] = [
     [
       {
         name: "foo",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\$)[^\\/#\\?])+?",
         prefix: "$",
         suffix: "",
         modifier: "",
       },
       {
         name: "bar",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!\\$)[^\\/#\\?])+?",
         prefix: "$",
         suffix: "",
         modifier: "?",
@@ -2392,14 +2392,14 @@ const TESTS: Test[] = [
       },
       {
         name: "attr2",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!-)[^\\/#\\?])+?",
         prefix: "-",
         suffix: "",
         modifier: "?",
       },
       {
         name: "attr3",
-        pattern: "[^\\/#\\?]+?",
+        pattern: "(?:(?!-)[^\\/#\\?])+?",
         prefix: "-",
         suffix: "",
         modifier: "?",
@@ -2597,39 +2597,6 @@ const TESTS: Test[] = [
       [{ foo: "#" }, null],
     ],
   ],
-  /**
-   * https://github.com/pillarjs/path-to-regexp/issues/260
-   */
-  [
-    ":name*",
-    undefined,
-    [
-      {
-        name: "name",
-        prefix: "",
-        suffix: "",
-        modifier: "*",
-        pattern: "[^\\/#\\?]+?",
-      },
-    ],
-    [["foobar", ["foobar", "foobar"]]],
-    [[{ name: "foobar" }, "foobar"]],
-  ],
-  [
-    ":name+",
-    undefined,
-    [
-      {
-        name: "name",
-        prefix: "",
-        suffix: "",
-        modifier: "+",
-        pattern: "[^\\/#\\?]+?",
-      },
-    ],
-    [["foobar", ["foobar", "foobar"]]],
-    [[{ name: "foobar" }, "foobar"]],
-  ],
 ];
 
 /**

diff --git a/src/index.ts b/src/index.ts
@@ -139,8 +139,7 @@ export interface ParseOptions {
  */
 export function parse(str: string, options: ParseOptions = {}): Token[] {
   const tokens = lexer(str);
-  const { prefixes = "./" } = options;
-  const defaultPattern = `[^${escapeString(options.delimiter || "/#?")}]+?`;
+  const { prefixes = "./", delimiter = "/#?" } = options;
   const result: Token[] = [];
   let key = 0;
   let i = 0;
@@ -166,6 +165,25 @@ export function parse(str: string, options: ParseOptions = {}): Token[] {
     return result;
   };
 
+  const isSafe = (value: string): boolean => {
+    for (const char of delimiter) if (value.indexOf(char) > -1) return true;
+    return false;
+  };
+
+  const safePattern = (prefix: string) => {
+    const prev = result[result.length - 1];
+    const prevText = prefix || (prev && typeof prev === "string" ? prev : "");
+
+    if (prev && !prevText) {
+      throw new TypeError(
+        `No support for parameters without text between them after "${(prev as Key).name}"`,
+      );
+    }
+
+    if (!prevText || isSafe(prevText)) return `[^${escapeString(delimiter)}]+?`;
+    return `(?:(?!${escapeString(prevText)})[^${escapeString(delimiter)}])+?`;
+  };
+
   while (i < tokens.length) {
     const char = tryConsume("CHAR");
     const name = tryConsume("NAME");
@@ -188,7 +206,7 @@ export function parse(str: string, options: ParseOptions = {}): Token[] {
         name: name || key++,
         prefix,
         suffix: "",
-        pattern: pattern || defaultPattern,
+        pattern: pattern || safePattern(prefix),
         modifier: tryConsume("MODIFIER") || "",
       });
       continue;
@@ -216,7 +234,7 @@ export function parse(str: string, options: ParseOptions = {}): Token[] {
 
       result.push({
         name: name || (pattern ? key++ : ""),
-        pattern: name && !pattern ? defaultPattern : pattern,
+        pattern: name && !pattern ? safePattern(prefix) : pattern,
         prefix,
         suffix,
         modifier: tryConsume("MODIFIER") || "",
@@ -564,10 +582,12 @@ export function tokensToRegexp(
           }
         } else {
           if (token.modifier === "+" || token.modifier === "*") {
-            route += `((?:${token.pattern})${token.modifier})`;
-          } else {
-            route += `(${token.pattern})${token.modifier}`;
+            throw new TypeError(
+              `Can not repeat ${token.name} with no prefix or suffix, e.g. "/:param${token.modifier}"`,
+            );
           }
+
+          route += `(${token.pattern})${token.modifier}`;
         }
       } else {
         route += `(?:${prefix}${suffix})${token.modifier}`;