avoid double-slash redirects (#3222)

deployer/aws-lambda/content-origin-request/index.js

@@ -78,6 +78,27 @@ exports.handler = async (event) => {
   const host = request.headers.host[0].value.toLowerCase();
   const qs = request.querystring ? `?${request.querystring}` : "";
 
+  // If the URL was something like `https://domain/en-US/search/`, our code
+  // would make a that a redirect to `/en-US/search` (stripping the trailing slash).
+  // But if it was `https://domain/en-US/search/` it'd make a redirect
+  // to `//en-US/search`.
+  // However, if pathname starts with `//` the Location header might look
+  // relative but it's actually an absolute URL.
+  // A 302 redirect from `https://domain//evil.com/` actually ends open
+  // opening `https://evil.com/` in the browser, because the browser will
+  // treat `//evil.com/ == https://evil.com/`.
+  // Prevent any pathnames that start with a double //.
+  if (request.uri.startsWith("//")) {
+    return {
+      status: 404,
+      statusDescription: "Not found",
+      headers: {
+        "content-type": [{ key: "Content-Type", value: "text/plain" }],
+      },
+      body: "URL pathname can't start with //\n",
+    };
+  }
+
   const { url, status } = resolveFundamental(request.uri);
   if (url) {
     // TODO: Do we want to add the query string to the redirect?

deployer/aws-lambda/content-origin-request/server.js

@@ -43,9 +43,6 @@ async function catchall(req, res) {
   };
 
   event.Records = [{ cf }];
-  // console.log("EVENT...");
-  // console.log(event);
-  // console.log(JSON.stringify(event, null, 3));
   const handle = await handler(event);
   if (handle === cf.request) {
     // The request is allowed to pass through.
@@ -59,26 +56,39 @@ async function catchall(req, res) {
     res.end(`${msg}\n`);
   } else if (handle.status) {
     // It's a redirect.
-    // console.log(JSON.stringify(handle, null, 3));
-    let path = null;
-    for (const headers of Object.values(handle.headers)) {
+    let location = null;
+    for (const headers of Object.values(handle.headers || {})) {
       for (const header of headers) {
         res.setHeader(header.key, header.value);
         if (header.key.toLowerCase() === "location") {
-          path = header.value;
+          location = header.value;
         }
       }
     }
-    console.log(
-      `${
-        handle.status === 302
-          ? kleur.green(handle.status)
-          : kleur.yellow(handle.status)
-      } redirect to ${kleur.bold(path)}`
-    );
+    if (handle.status >= 300 && handle.status < 400) {
+      console.log(
+        `${
+          handle.status === 302
+            ? kleur.green(handle.status)
+            : kleur.yellow(handle.status)
+        } redirect to ${kleur.bold(location)}`
+      );
+      if (location) {
+        res.setHeader("Location", location);
+      }
+    } else if (handle.status >= 400) {
+      console.log(
+        `${
+          handle.status >= 500
+            ? kleur.red(handle.status)
+            : kleur.yellow(handle.status)
+        } '${handle.body.trim()}'`
+      );
+    } else {
+      console.log(`${kleur.gree(handle.status)} '${handle.body.trim()}'`);
+    }
     res.statusCode = handle.status;
-    res.setHeader("Location", path);
-    res.end("");
+    res.end(handle.body || "");
   } else {
     console.warn(JSON.stringify(handle, null, 3));
     res.statusCode = 500;

deployer/aws-lambda/content-origin-request/server.test.js

@@ -13,6 +13,7 @@ async function get(uri, headers = {}) {
     headers,
     followRedirect: false,
     retry: 0,
+    throwHttpErrors: false,
   });
   return response;
 }
@@ -186,3 +187,12 @@ describe("always check for fundamental redirects first", () => {
     }
   });
 });
+
+describe("avoid double-slash redirects", () => {
+  it("should 404 on any pathname that starts with //", async () => {
+    const r = await get(`//en-US/search/`);
+    expect(r.statusCode).toBe(404);
+    expect(r.headers["location"]).toBeFalsy();
+    expect(r.body).toContain("URL pathname can't start with //");
+  });
+});