diff --git a/app/routes/join.tsx b/app/routes/join.tsx index 44a14d2b..a6e45c1e 100644 --- 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( @@ -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 index a725365e..34121655 100644 --- 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 index d0238204..a7eb66eb 100644 --- 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.