fix: prevent OAuth parameter injection via returnTo (#2381) (#2413)

Author: tusharpandey13

.gitignore

@@ -141,4 +141,5 @@ dist
 *.tmp
 *PLAN*.md
 .yalc/
-yalc.lock
+yalc.lock
+.npmrc

src/server/helpers/with-page-auth-required.test.ts

@@ -81,7 +81,7 @@ describe("with-page-auth-required ssr", () => {
       );
       await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
       expect(redirect).toHaveBeenCalledTimes(1);
-      expect(redirect).toHaveBeenCalledWith("/auth/login?returnTo=/foo");
+      expect(redirect).toHaveBeenCalledWith("/auth/login?returnTo=%2Ffoo");
     });
 
     it("should protect a page and redirect to returnTo fn option", async () => {
@@ -114,7 +114,7 @@ describe("with-page-auth-required ssr", () => {
       ).rejects.toThrowError("NEXT_REDIRECT");
       expect(redirect).toHaveBeenCalledTimes(1);
       expect(redirect).toHaveBeenCalledWith(
-        "/auth/login?returnTo=/foo/bar?foo=bar"
+        "/auth/login?returnTo=%2Ffoo%2Fbar%3Ffoo%3Dbar"
       );
     });
 
@@ -165,6 +165,91 @@ describe("with-page-auth-required ssr", () => {
       expect(redirect).toHaveBeenCalledTimes(1);
       expect(redirect).toHaveBeenCalledWith("/api/auth/custom-login");
     });
+
+    it("should URL encode returnTo parameter to prevent OAuth param injection", async () => {
+      const withPageAuthRequired = appRouteHandlerFactory(
+        new Auth0Client({
+          domain: constants.domain,
+          clientId: constants.clientId,
+          clientSecret: constants.clientSecret,
+          appBaseUrl: constants.appBaseUrl,
+          secret: constants.secret
+        }),
+        {
+          loginUrl: "/auth/login"
+        }
+      );
+      const handler = withPageAuthRequired(
+        () => Promise.resolve(React.createElement("div", {}, "foo")),
+        {
+          returnTo:
+            "/callback?scope=openid profile&audience=https://api.example.com"
+        }
+      );
+      await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
+      expect(redirect).toHaveBeenCalledTimes(1);
+      expect(redirect).toHaveBeenCalledWith(
+        "/auth/login?returnTo=%2Fcallback%3Fscope%3Dopenid%20profile%26audience%3Dhttps%3A%2F%2Fapi.example.com"
+      );
+    });
+
+    it("should URL encode returnTo with special characters", async () => {
+      const withPageAuthRequired = appRouteHandlerFactory(
+        new Auth0Client({
+          domain: constants.domain,
+          clientId: constants.clientId,
+          clientSecret: constants.clientSecret,
+          appBaseUrl: constants.appBaseUrl,
+          secret: constants.secret
+        }),
+        {
+          loginUrl: "/auth/login"
+        }
+      );
+      const handler = withPageAuthRequired(
+        () => Promise.resolve(React.createElement("div", {}, "foo")),
+        {
+          returnTo: "/path/with spaces & special=chars"
+        }
+      );
+      await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
+      expect(redirect).toHaveBeenCalledTimes(1);
+      expect(redirect).toHaveBeenCalledWith(
+        "/auth/login?returnTo=%2Fpath%2Fwith%20spaces%20%26%20special%3Dchars"
+      );
+    });
+
+    it("should URL encode returnTo from function to prevent OAuth param injection", async () => {
+      const withPageAuthRequired = appRouteHandlerFactory(
+        new Auth0Client({
+          domain: constants.domain,
+          clientId: constants.clientId,
+          clientSecret: constants.clientSecret,
+          appBaseUrl: constants.appBaseUrl,
+          secret: constants.secret
+        }),
+        {
+          loginUrl: "/auth/login"
+        }
+      );
+      const handler = withPageAuthRequired(
+        () => Promise.resolve(React.createElement("div", {}, "foo")),
+        {
+          async returnTo({ params }: any) {
+            return `/callback/${(await params).id}?malicious=scope%3Dopenid`;
+          }
+        }
+      );
+      await expect(
+        handler({
+          params: Promise.resolve({ id: "123" })
+        })
+      ).rejects.toThrowError("NEXT_REDIRECT");
+      expect(redirect).toHaveBeenCalledTimes(1);
+      expect(redirect).toHaveBeenCalledWith(
+        "/auth/login?returnTo=%2Fcallback%2F123%3Fmalicious%3Dscope%253Dopenid"
+      );
+    });
   });
 
   describe("pages router", () => {

src/server/helpers/with-page-auth-required.ts

@@ -196,7 +196,7 @@ export const appRouteHandlerFactory =
           : opts.returnTo;
       const { redirect } = await import("next/navigation.js");
       redirect(
-        `${config.loginUrl}${opts.returnTo ? `?returnTo=${returnTo}` : ""}`
+        `${config.loginUrl}${returnTo ? `?returnTo=${encodeURIComponent(returnTo)}` : ""}`
       );
     }
     return handler(params);