Add backtrack protection to 6.x (#324)

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

Hide file tree

Showing 5 changed files with 184 additions and 55 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"
@@ -46,7 +47,7 @@
   "size-limit": [
     {
       "path": "dist.es2015/index.js",
-      "limit": "2 kB"
+      "limit": "2.1 kB"
     }
   ],
   "ts-scripts": {

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"]],
-  ],
 ];
 
 /**
@@ -2799,6 +2766,24 @@ describe("path-to-regexp", () => {
         pathToRegexp.pathToRegexp("/foo?");
       }).toThrow(new TypeError("Unexpected MODIFIER at 4, expected END"));
     });
+
+    it("should throw on parameters without text between them", () => {
+      expect(() => {
+        pathToRegexp.pathToRegexp("/:x:y");
+      }).toThrow(
+        new TypeError(
+          `Must have text between two parameters, missing text after "x"`,
+        ),
+      );
+    });
+
+    it("should throw on unrepeatable params", () => {
+      expect(() => {
+        pathToRegexp.pathToRegexp("/foo:x*");
+      }).toThrow(
+        new TypeError(`Can not repeat "x" without a prefix and suffix`),
+      );
+    });
   });
 
   describe("tokens", () => {