fix: open redirect issue with trailing slash detection (#2313)

Passing a path like `/foo/bar` as the location header is interpreted as
a relative path. But when it contains two leading slashes like
`//evil.com/` it is interpreted as a port relative url. This has the
consequence that the origin of a URL can essentially be replaced:

```ts
new URL("//evil.com", "https://example.com/");
// -> 'https://evil.com/'
```

Our trailing slash detection logic didn't guard against this case. By
normalizing the pathname of the URL and stripping sibling forward
slashes we sidestep this problem.

Author: marvinhagemeister

src/server/context.ts

@@ -187,6 +187,11 @@ export class ServerContext {
       connInfo: ServeHandlerInfo = DEFAULT_CONN_INFO,
     ) {
       const url = new URL(req.url);
+      // Syntactically having double slashes in the pathname is valid per
+      // spec, but there is no behavior defined for that. Practically all
+      // servers normalize the pathname of a URL to not include double
+      // forward slashes.
+      url.pathname = url.pathname.replaceAll(/\/+/g, "/");
 
       const aliveUrl = basePath + ALIVE_URL;
 

tests/main_test.ts

@@ -232,27 +232,27 @@ Deno.test("redirect /pages/fresh/ to /pages/fresh", async () => {
   );
 });
 
-Deno.test("redirect /pages/////fresh///// to /pages/////fresh", async () => {
+Deno.test("redirect /pages/////fresh///// to /pages/fresh", async () => {
   const resp = await handler(
     new Request("https://fresh.deno.dev/pages/////fresh/////"),
   );
   assert(resp);
   assertEquals(resp.status, STATUS_CODE.TemporaryRedirect);
   assertEquals(
     resp.headers.get("location"),
-    "/pages/////fresh",
+    "/pages/fresh",
   );
 });
 
-Deno.test("redirect /pages/////fresh/ to /pages/////fresh", async () => {
+Deno.test("redirect /pages/////fresh/ to /pages/fresh", async () => {
   const resp = await handler(
     new Request("https://fresh.deno.dev/pages/////fresh/"),
   );
   assert(resp);
   assertEquals(resp.status, STATUS_CODE.TemporaryRedirect);
   assertEquals(
     resp.headers.get("location"),
-    "/pages/////fresh",
+    "/pages/fresh",
   );
 });
 
@@ -264,6 +264,15 @@ Deno.test("no redirect for /pages/////fresh", async () => {
   assertEquals(resp.status, STATUS_CODE.NotFound);
 });
 
+Deno.test("no open redirect when passing double slashes", async () => {
+  const resp = await handler(
+    new Request("https://fresh.deno.dev//evil.com/"),
+  );
+  assert(resp);
+  assertEquals(resp.status, STATUS_CODE.TemporaryRedirect);
+  assertEquals(resp.headers.get("location"), "/evil.com");
+});
+
 Deno.test("/failure", async () => {
   const resp = await handler(new Request("https://fresh.deno.dev/failure"));
   assert(resp);