fix: validate paths after stripping publicPath

src/utils/getFilenameFromUrl.js

@@ -118,33 +118,29 @@ function getFilenameFromUrl(context, url) {
         throw new FilenameError("Bad Request", 400);
       }
 
+      // Strip the public path prefix from the decoded pathname:
+      // `/complex/foo.js` => `foo.js`.
+      const relativePathname = pathname.slice(publicPathPathname.length);
+
       // Only decoded paths containing two consecutive dots can produce a
       // parent-directory segment through `path.join`.
-      const mayHaveUpPath = pathname.includes("..");
+      const mayHaveUpPath = relativePathname.includes("..");
 
-      // ".." is malicious
+      // Stripping a public path without a trailing slash can turn
+      // `/assets../secret` into `../secret`. Check the remainder before joining.
       if (
         mayHaveUpPath &&
-        UP_PATH_REGEXP.test(path.normalize(`./${pathname}`))
+        UP_PATH_REGEXP.test(path.normalize(`./${relativePathname}`))
       ) {
         throw new FilenameError("Forbidden", 403);
       }
 
-      // Strip the `pathname` property from the `publicPath` option from the start of requested url
-      // `/complex/foo.js` => `foo.js`
-      // and add outputPath
-      // `foo.js` => `/home/user/my-project/dist/foo.js`
-      filename = path.join(
-        outputPath,
-        pathname.slice(publicPathPathname.length),
-      );
+      // Add outputPath: `foo.js` => `/home/user/my-project/dist/foo.js`.
+      filename = path.join(outputPath, relativePathname);
 
       if (mayHaveUpPath) {
-        // A public path without a trailing slash can consume the characters
-        // before `..`, turning a safe-looking pathname such as
-        // `/assets../secret` into `../secret` after the prefix is stripped.
-        // Validate the final candidate while keeping ordinary requests on the
-        // fast path.
+        // Keep the final containment check as a backstop for platform-specific
+        // path behavior, without adding work for ordinary requests.
         const relative = path.relative(outputPath, filename);
 
         if (relative === ".." || relative.startsWith(`..${path.sep}`)) {
@@ -163,9 +159,8 @@ function getFilenameFromUrl(context, url) {
         // Lazy evaluate immutable because it may be expensive to get asset info
         get immutable() {
           if (immutable === undefined) {
-            const assetName = pathname.slice(publicPathPathname.length);
             immutable = Boolean(
-              compilation.getAsset(assetName)?.info?.immutable,
+              compilation.getAsset(relativePathname)?.info?.immutable,
             );
           }
           return immutable;

test/middleware.test.js

@@ -990,6 +990,12 @@ describe.each([
           expect(response.statusCode).toBe(403);
           expect(response.text).not.toContain("TOP SECRET");
         });
+
+        it("should reject a path that leaves and re-enters the output path", async () => {
+          const response = await req.get("/static../dist/index.html");
+
+          expect(response.statusCode).toBe(403);
+        });
       });
 
       describe("should work", () => {