From 6ef3ec6b56ddb1c9c51e9a6c9648de0b0f8c1777 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Fri, 11 Oct 2024 13:08:28 +0300 Subject: [PATCH] fix(clerk-js): Update isAllowedRedirectOrigin to handle malformed or protocol-relative URLs (#4317) --- .changeset/blue-pumpkins-travel.md | 5 +++ .../clerk-js/src/utils/__tests__/url.test.ts | 6 +++ packages/clerk-js/src/utils/url.ts | 40 +++++++++---------- 3 files changed, 31 insertions(+), 20 deletions(-) create mode 100644 .changeset/blue-pumpkins-travel.md diff --git a/.changeset/blue-pumpkins-travel.md b/.changeset/blue-pumpkins-travel.md new file mode 100644 index 0000000000..13cbb83449 --- /dev/null +++ b/.changeset/blue-pumpkins-travel.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +Correctly handle malformed or protocol-relative URLs before navigating to cross-origin URLs diff --git a/packages/clerk-js/src/utils/__tests__/url.test.ts b/packages/clerk-js/src/utils/__tests__/url.test.ts index 79b9a8a7d6..6d90443ddc 100644 --- a/packages/clerk-js/src/utils/__tests__/url.test.ts +++ b/packages/clerk-js/src/utils/__tests__/url.test.ts @@ -472,6 +472,12 @@ describe('isAllowedRedirectOrigin', () => { // regexp ['https://www.clerk.com/foo?hello=1', [/https:\/\/www\.clerk\.com/], true], ['https://test.clerk.com/foo?hello=1', [/https:\/\/www\.clerk\.com/], false], + // malformed or protocol-relative URLs + ['http:evil.com', [/https:\/\/www\.clerk\.com/], false], + ['https:evil.com', [/https:\/\/www\.clerk\.com/], false], + ['http//evil.com', [/https:\/\/www\.clerk\.com/], false], + ['https//evil.com', [/https:\/\/www\.clerk\.com/], false], + ['//evil.com', [/https:\/\/www\.clerk\.com/], false], ]; const warnMock = jest.spyOn(logger, 'warnOnce'); diff --git a/packages/clerk-js/src/utils/url.ts b/packages/clerk-js/src/utils/url.ts index b5bd222624..971d670123 100644 --- a/packages/clerk-js/src/utils/url.ts +++ b/packages/clerk-js/src/utils/url.ts @@ -241,15 +241,29 @@ export function relativeToAbsoluteUrl(url: string, origin: string | URL): string return new URL(url, origin).href; } -export function isRelativeUrl(val: unknown): val is string { +export function isRelativeUrl(val: string): boolean { if (val !== val && !val) { return false; } + + if (val.startsWith('//') || val.startsWith('http/') || val.startsWith('https/')) { + // Protocol-relative URL; consider it absolute. + return false; + } + try { - const temp = new URL(val as string, DUMMY_URL_BASE); - return temp.origin === DUMMY_URL_BASE; - } catch (e) { + // If this does not throw, it's a valid absolute URL + new URL(val); return false; + } catch (e) { + try { + // If this does not throw, it's a valid relative URL + new URL(val, DUMMY_URL_BASE); + return true; + } catch (e) { + // Invalid URL case + return false; + } } } @@ -344,12 +358,11 @@ export const isAllowedRedirectOrigin = return true; } - const url = new URL(_url, DUMMY_URL_BASE); - const isRelativeUrl = url.origin === DUMMY_URL_BASE; - if (isRelativeUrl) { + if (isRelativeUrl(_url)) { return true; } + const url = new URL(_url, DUMMY_URL_BASE); const isAllowed = allowedRedirectOrigins .map(origin => (typeof origin === 'string' ? globs.toRegexp(trimTrailingSlash(origin)) : origin)) .some(origin => origin.test(trimTrailingSlash(url.origin))); @@ -380,16 +393,3 @@ export function createAllowedRedirectOrigins( return origins; } - -export const isCrossOrigin = (url: string | URL, origin: string | URL = window.location.origin): boolean => { - try { - if (isRelativeUrl(url)) { - return false; - } - const urlOrigin = new URL(url).origin; - const originOrigin = new URL(origin).origin; - return urlOrigin !== originOrigin; - } catch (e) { - return false; - } -};