Security fixes: session hardening, open redirect, SSR path injection #1

Merged
ArdaSamadi merged 1 commits from security-fixes into main 2026-06-14 15:58:50 +00:00
5 changed files with 36 additions and 8 deletions
Showing only changes of commit 485f8d460a - Show all commits

View File

@ -22,9 +22,23 @@ import { useToast } from "~/hooks/use-toast";
import { useSubmit, useLoaderData } from "@remix-run/react"; import { useSubmit, useLoaderData } from "@remix-run/react";
import { User } from "~/utils/token-manager.server"; import { User } from "~/utils/token-manager.server";
// Only allow same-site, path-relative redirects. Rejects absolute URLs
// (https://evil.com) and protocol-relative URLs (//evil.com) to prevent
// open-redirect / phishing after login.
function safeRedirect(
to: FormDataEntryValue | string | null,
defaultRedirect = "/"
): string {
if (!to || typeof to !== "string") return defaultRedirect;
if (!to.startsWith("/") || to.startsWith("//") || to.startsWith("/\\")) {
return defaultRedirect;
}
return to;
}
export async function loader({ request }: LoaderFunctionArgs) { export async function loader({ request }: LoaderFunctionArgs) {
const url = new URL(request.url); const url = new URL(request.url);
const redirectTo = url.searchParams.get("redirectTo") || "/"; const redirectTo = safeRedirect(url.searchParams.get("redirectTo"));
return json({ redirectTo }); return json({ redirectTo });
} }
@ -290,7 +304,7 @@ export const meta: MetaFunction = () => {
export async function action({ request }: ActionFunctionArgs) { export async function action({ request }: ActionFunctionArgs) {
const clonedRequest = request.clone(); const clonedRequest = request.clone();
const formData = await clonedRequest.formData(); const formData = await clonedRequest.formData();
const redirectTo = (formData.get("redirectTo") as string) || "/"; const redirectTo = safeRedirect(formData.get("redirectTo"));
try { try {
const user = (await authenticator.authenticate( const user = (await authenticator.authenticate(
@ -315,7 +329,7 @@ export async function action({ request }: ActionFunctionArgs) {
refresh_token_timestamp: currentTime, refresh_token_timestamp: currentTime,
}); });
return redirect(redirectTo || "/", { return redirect(redirectTo, {
headers: { headers: {
"Set-Cookie": await sessionStorage.commitSession(session, { "Set-Cookie": await sessionStorage.commitSession(session, {
expires: new Date(new Date().setMonth(new Date().getMonth() + 1)), expires: new Date(new Date().setMonth(new Date().getMonth() + 1)),

View File

@ -20,7 +20,7 @@ export async function loader({ params }: LoaderFunctionArgs) {
const { id } = params; const { id } = params;
try { try {
const response = await fetch( const response = await fetch(
`${getInternalApiUrl()}/api/product/v1/products/${id}/` `${getInternalApiUrl()}/api/product/v1/products/${encodeURIComponent(id ?? "")}/`
); );
if (!response.ok) { if (!response.ok) {

View File

@ -59,7 +59,7 @@ export async function loader({ params }: LoaderFunctionArgs) {
throw new Response("Seller ID is required", { status: 400 }); throw new Response("Seller ID is required", { status: 400 });
} }
try { try {
const response = await fetch(`${getInternalApiUrl()}/seller/${sellerId}/`); const response = await fetch(`${getInternalApiUrl()}/seller/${encodeURIComponent(sellerId)}/`);
if (!response.ok) { if (!response.ok) {
throw new Error(`Failed to fetch seller: ${response.status}`); throw new Error(`Failed to fetch seller: ${response.status}`);

View File

@ -71,7 +71,7 @@ export async function loader({ params }: LoaderFunctionArgs) {
const { id } = params; const { id } = params;
try { try {
const response = await fetch( const response = await fetch(
`${getInternalApiUrl()}/api/product/v1/products/${id}/`, `${getInternalApiUrl()}/api/product/v1/products/${encodeURIComponent(id ?? "")}/`,
); );
if (!response.ok) { if (!response.ok) {

View File

@ -1,6 +1,17 @@
import { createCookieSessionStorage, Session } from "@remix-run/node"; import { createCookieSessionStorage, Session } from "@remix-run/node";
import { isProduction } from "./utils/get-env"; import { isProduction } from "./utils/get-env";
const isProd = process.env.NODE_ENV === "production";
const sessionSecret = process.env.REMIX_SECRET;
// Fail closed: never ship a guessable signing secret to production. A signed
// cookie with a known secret can be forged, allowing session tampering.
if (isProd && !sessionSecret) {
throw new Error(
"REMIX_SECRET environment variable must be set in production."
);
}
// export the whole sessionStorage object // export the whole sessionStorage object
export const sessionStorage = createCookieSessionStorage({ export const sessionStorage = createCookieSessionStorage({
cookie: { cookie: {
@ -8,8 +19,11 @@ export const sessionStorage = createCookieSessionStorage({
sameSite: "lax", // this helps with CSRF sameSite: "lax", // this helps with CSRF
path: "/", // remember to add this so the cookie will work in all routes path: "/", // remember to add this so the cookie will work in all routes
httpOnly: true, // for security reasons, make this cookie http only httpOnly: true, // for security reasons, make this cookie http only
secrets: [process.env.REMIX_SECRET ?? "secret"], // replace this with an actual secret maxAge: 60 * 60 * 24 * 30, // 30 days (matches refresh-token lifetime)
secure: isProduction(), // enable this in prod only // Dev-only fallback; production is guarded by the check above.
secrets: [sessionSecret ?? "dev-only-insecure-secret"],
// Send only over HTTPS in production (NODE_ENV is the reliable server signal).
secure: isProd || isProduction(),
}, },
}); });
export const { getSession, commitSession, destroySession } = sessionStorage; export const { getSession, commitSession, destroySession } = sessionStorage;