fix: open redirects (remix-run#68)

Co-authored-by: Kent C. Dodds <[email protected]>
chiangs · Apr 20, 2022 · 7a012f4 · 7a012f4
1 parent e4c669c
commit 7a012f4
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 6 deletions.
diff --git a/app/routes/join.tsx b/app/routes/join.tsx
@@ -10,7 +10,7 @@ import * as React from "react";
 import { getUserId, createUserSession } from "~/session.server";
 
 import { createUser, getUserByEmail } from "~/models/user.server";
-import { validateEmail } from "~/utils";
+import { safeRedirect, validateEmail } from "~/utils";
 
 export const loader: LoaderFunction = async ({ request }) => {
   const userId = await getUserId(request);
@@ -29,7 +29,7 @@ export const action: ActionFunction = async ({ request }) => {
   const formData = await request.formData();
   const email = formData.get("email");
   const password = formData.get("password");
-  const redirectTo = formData.get("redirectTo");
+  const redirectTo = safeRedirect(formData.get("redirectTo"), "/");
 
   if (!validateEmail(email)) {
     return json<ActionData>(
@@ -66,7 +66,7 @@ export const action: ActionFunction = async ({ request }) => {
     request,
     userId: user.id,
     remember: false,
-    redirectTo: typeof redirectTo === "string" ? redirectTo : "/",
+    redirectTo,
   });
 };
 

diff --git a/app/routes/login.tsx b/app/routes/login.tsx
@@ -9,7 +9,7 @@ import * as React from "react";
 
 import { createUserSession, getUserId } from "~/session.server";
 import { verifyLogin } from "~/models/user.server";
-import { validateEmail } from "~/utils";
+import { safeRedirect, validateEmail } from "~/utils";
 
 export const loader: LoaderFunction = async ({ request }) => {
   const userId = await getUserId(request);
@@ -28,7 +28,7 @@ export const action: ActionFunction = async ({ request }) => {
   const formData = await request.formData();
   const email = formData.get("email");
   const password = formData.get("password");
-  const redirectTo = formData.get("redirectTo");
+  const redirectTo = safeRedirect(formData.get("redirectTo"), "/notes");
   const remember = formData.get("remember");
 
   if (!validateEmail(email)) {
@@ -65,7 +65,7 @@ export const action: ActionFunction = async ({ request }) => {
     request,
     userId: user.id,
     remember: remember === "on" ? true : false,
-    redirectTo: typeof redirectTo === "string" ? redirectTo : "/notes",
+    redirectTo,
   });
 };
 

diff --git a/app/utils.ts b/app/utils.ts
@@ -3,6 +3,30 @@ import { useMemo } from "react";
 
 import type { User } from "~/models/user.server";
 
+const DEFAULT_REDIRECT = "/";
+
+/**
+ * This should be used any time the redirect path is user-provided
+ * (Like the query string on our login/signup pages). This avoids
+ * open-redirect vulnerabilities.
+ * @param {string} to The redirect destination
+ * @param {string} defaultRedirect The redirect to use if the to is unsafe.
+ */
+export function safeRedirect(
+  to: FormDataEntryValue | string | null | undefined,
+  defaultRedirect: string = DEFAULT_REDIRECT
+) {
+  if (!to || typeof to !== "string") {
+    return defaultRedirect;
+  }
+
+  if (!to.startsWith("/") || to.startsWith("//")) {
+    return defaultRedirect;
+  }
+
+  return to;
+}
+
 /**
  * This base hook is used in other hooks to quickly search for specific data
  * across all loader data using useMatches.