From 96289bf555cba528393d18695718fe3128130289 Mon Sep 17 00:00:00 2001 From: Patrik Date: Mon, 31 Mar 2025 13:11:34 -0400 Subject: [PATCH] fix(next): block encoded and escaped open redirects in getSafeRedirect (#11907) ### What This PR improves the `getSafeRedirect` utility to improve security around open redirect handling. ### How - Normalizes and decodes the redirect path using `decodeURIComponent` - Catches malformed encodings with a try/catch fallback - Blocks open redirects --- .../src/utilities/getSafeRedirect.spec.ts | 55 +++++++++++++++++++ .../next/src/utilities/getSafeRedirect.ts | 17 +++++- test/admin-root/payload-types.ts | 1 + 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 packages/next/src/utilities/getSafeRedirect.spec.ts diff --git a/packages/next/src/utilities/getSafeRedirect.spec.ts b/packages/next/src/utilities/getSafeRedirect.spec.ts new file mode 100644 index 0000000000..e146e9a33f --- /dev/null +++ b/packages/next/src/utilities/getSafeRedirect.spec.ts @@ -0,0 +1,55 @@ +import { getSafeRedirect } from './getSafeRedirect' + +const fallback = '/admin' // default fallback if the input is unsafe or invalid + +describe('getSafeRedirect', () => { + // Valid - safe redirect paths + it.each([['/dashboard'], ['/admin/settings'], ['/projects?id=123'], ['/hello-world']])( + 'should allow safe relative path: %s', + (input) => { + // If the input is a clean relative path, it should be returned as-is + expect(getSafeRedirect(input, fallback)).toBe(input) + }, + ) + + // Invalid types or empty inputs + it.each(['', null, undefined, 123, {}, []])( + 'should fallback on invalid or non-string input: %s', + (input) => { + // If the input is not a valid string, it should return the fallback + expect(getSafeRedirect(input as any, fallback)).toBe(fallback) + }, + ) + + // Unsafe redirect patterns + it.each([ + '//example.com', // protocol-relative URL + '/javascript:alert(1)', // JavaScript scheme + '/JavaScript:alert(1)', // case-insensitive JavaScript + '/http://unknown.com', // disguised external redirect + '/https://unknown.com', // disguised external redirect + '/%2Funknown.com', // encoded slash — could resolve to // + '/\\/unknown.com', // escaped slash + '/\\\\unknown.com', // double escaped slashes + '/\\unknown.com', // single escaped slash + '%2F%2Funknown.com', // fully encoded protocol-relative path + '%2Fjavascript:alert(1)', // encoded JavaScript scheme + ])('should block unsafe redirect: %s', (input) => { + // All of these should return the fallback because they’re unsafe + expect(getSafeRedirect(input, fallback)).toBe(fallback) + }) + + // Input with extra spaces should still be properly handled + it('should trim whitespace before evaluating', () => { + // A valid path with surrounding spaces should still be accepted + expect(getSafeRedirect(' /dashboard ', fallback)).toBe('/dashboard') + + // An unsafe path with spaces should still be rejected + expect(getSafeRedirect(' //example.com ', fallback)).toBe(fallback) + }) + + // If decoding the input fails (e.g., invalid percent encoding), it should not crash + it('should return fallback on invalid encoding', () => { + expect(getSafeRedirect('%E0%A4%A', fallback)).toBe(fallback) + }) +}) diff --git a/packages/next/src/utilities/getSafeRedirect.ts b/packages/next/src/utilities/getSafeRedirect.ts index 963c6b51fe..4770d7fe88 100644 --- a/packages/next/src/utilities/getSafeRedirect.ts +++ b/packages/next/src/utilities/getSafeRedirect.ts @@ -6,14 +6,25 @@ export const getSafeRedirect = ( return fallback } - // Ensures that any leading or trailing whitespace doesn’t affect the checks - const redirectPath = redirectParam.trim() + // Normalize and decode the path + let redirectPath: string + try { + redirectPath = decodeURIComponent(redirectParam.trim()) + } catch { + return fallback // invalid encoding + } const isSafeRedirect = // Must start with a single forward slash (e.g., "/admin") redirectPath.startsWith('/') && - // Prevent protocol-relative URLs (e.g., "//evil.com") + // Prevent protocol-relative URLs (e.g., "//example.com") !redirectPath.startsWith('//') && + // Prevent encoded slashes that could resolve to protocol-relative + !redirectPath.startsWith('/%2F') && + // Prevent backslash-based escape attempts (e.g., "/\\/example.com", "/\\\\example.com", "/\\example.com") + !redirectPath.startsWith('/\\/') && + !redirectPath.startsWith('/\\\\') && + !redirectPath.startsWith('/\\') && // Prevent javascript-based schemes (e.g., "/javascript:alert(1)") !redirectPath.toLowerCase().startsWith('/javascript:') && // Prevent attempts to redirect to full URLs using "/http:" or "/https:" diff --git a/test/admin-root/payload-types.ts b/test/admin-root/payload-types.ts index ab6df309d8..4ca79e12da 100644 --- a/test/admin-root/payload-types.ts +++ b/test/admin-root/payload-types.ts @@ -54,6 +54,7 @@ export type SupportedTimezones = | 'Asia/Singapore' | 'Asia/Tokyo' | 'Asia/Seoul' + | 'Australia/Brisbane' | 'Australia/Sydney' | 'Pacific/Guam' | 'Pacific/Noumea'