host parse (#10553)

Author: jacob-ebey

.changeset/old-plums-perform.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/express": patch
+---
+
+Better validation of `x-forwarded-host` header to preent potential security issues.

packages/remix-express/__tests__/server-test.ts

@@ -219,4 +219,28 @@ describe("express createRemixRequest", () => {
     );
     expect(remixRequest.headers.get("host")).toBe("localhost:3000");
   });
+
+  it("validates parsed port", async () => {
+    let expressRequest = createRequest({
+      url: "/foo/bar",
+      method: "GET",
+      protocol: "http",
+      hostname: "localhost",
+      headers: {
+        "Cache-Control": "max-age=300, s-maxage=3600",
+        Host: "localhost:3000",
+        "x-forwarded-host": ":/spoofed",
+      },
+    });
+    let expressResponse = createResponse();
+
+    let remixRequest = createRemixRequest(expressRequest, expressResponse);
+
+    expect(remixRequest.method).toBe("GET");
+    expect(remixRequest.headers.get("cache-control")).toBe(
+      "max-age=300, s-maxage=3600"
+    );
+    expect(remixRequest.headers.get("host")).toBe("localhost:3000");
+    expect(remixRequest.url).toBe("http://localhost:3000/foo/bar");
+  });
 });

packages/remix-express/server.ts

@@ -89,9 +89,15 @@ export function createRemixRequest(
 ): Request {
   // req.hostname doesn't include port information so grab that from
   // `X-Forwarded-Host` or `Host`
-  let [, hostnamePort] = req.get("X-Forwarded-Host")?.split(":") ?? [];
-  let [, hostPort] = req.get("host")?.split(":") ?? [];
-  let port = hostnamePort || hostPort;
+  let [, hostnamePortStr] = req.get("X-Forwarded-Host")?.split(":") ?? [];
+  let [, hostPortStr] = req.get("host")?.split(":") ?? [];
+  let hostnamePort = Number.parseInt(hostnamePortStr, 10);
+  let hostPort = Number.parseInt(hostPortStr, 10);
+  let port = Number.isSafeInteger(hostnamePort)
+    ? hostnamePort
+    : Number.isSafeInteger(hostPort)
+    ? hostPort
+    : "";
   // Use req.hostname here as it respects the "trust proxy" setting
   let resolvedHost = `${req.hostname}${port ? `:${port}` : ""}`;
   // Use `req.originalUrl` so Remix is aware of the full path