From 13abc7922b47f800da59ca212d364a64feeec91f Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Sun, 31 May 2026 13:37:39 +0100 Subject: [PATCH] fix(core): make redirect-uri validation runtime-safe and reject fragments (#9845) --- .changeset/safe-url-runtime-fragment.md | 7 +++++++ packages/core/src/utils/redirect-uri.ts | 15 ++++++++++++--- packages/core/src/utils/url.test.ts | 12 ++++++++++++ packages/core/src/utils/url.ts | 7 +++++-- 4 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 .changeset/safe-url-runtime-fragment.md diff --git a/.changeset/safe-url-runtime-fragment.md b/.changeset/safe-url-runtime-fragment.md new file mode 100644 index 0000000000..030722dcb3 --- /dev/null +++ b/.changeset/safe-url-runtime-fragment.md @@ -0,0 +1,7 @@ +--- +"@better-auth/oauth-provider": patch +"@better-auth/core": patch +"better-auth": patch +--- + +Harden redirect-URI validation across the OAuth provider plugins. `isSafeUrlScheme` and `SafeUrlSchema` no longer call `URL.canParse`, which is absent on some supported runtimes and could throw or silently disable the dangerous-scheme check. They now parse with a `try`/`catch` fallback. `SafeUrlSchema` also rejects redirect URIs that contain a fragment component, per RFC 6749 §3.1.2. diff --git a/packages/core/src/utils/redirect-uri.ts b/packages/core/src/utils/redirect-uri.ts index ec087eeb1d..fcee3aa2c8 100644 --- a/packages/core/src/utils/redirect-uri.ts +++ b/packages/core/src/utils/redirect-uri.ts @@ -7,6 +7,7 @@ import { DANGEROUS_URL_SCHEMES } from "./url"; * server stores and later hands back to a browser. * * - Rejects dangerous schemes (`javascript:`, `data:`, `vbscript:`). + * - Rejects URIs with a fragment component (`#...`) per RFC 6749 §3.1.2. * - Requires HTTPS, except for loopback hosts (`127.0.0.0/8`, `[::1]`, * `*.localhost` per RFC 6761), where HTTP is allowed for local development. * - Allows custom schemes for mobile apps (e.g. `myapp://callback`). @@ -16,7 +17,10 @@ import { DANGEROUS_URL_SCHEMES } from "./url"; * rather than re-implementing the scheme policy per plugin. */ export const SafeUrlSchema = z.url().superRefine((val, ctx) => { - if (!URL.canParse(val)) { + let u: URL; + try { + u = new URL(val); + } catch { ctx.addIssue({ code: "custom", message: "URL must be parseable", @@ -25,8 +29,6 @@ export const SafeUrlSchema = z.url().superRefine((val, ctx) => { return z.NEVER; } - const u = new URL(val); - if (DANGEROUS_URL_SCHEMES.includes(u.protocol)) { ctx.addIssue({ code: "custom", @@ -35,6 +37,13 @@ export const SafeUrlSchema = z.url().superRefine((val, ctx) => { return; } + if (val.includes("#")) { + ctx.addIssue({ + code: "custom", + message: "Redirect URI must not contain a fragment component", + }); + } + if (u.protocol === "http:" && !isLoopbackHost(u.host)) { ctx.addIssue({ code: "custom", diff --git a/packages/core/src/utils/url.test.ts b/packages/core/src/utils/url.test.ts index cd9a882e7b..f146471a59 100644 --- a/packages/core/src/utils/url.test.ts +++ b/packages/core/src/utils/url.test.ts @@ -46,4 +46,16 @@ describe("SafeUrlSchema", () => { ); expect(SafeUrlSchema.safeParse("http://127.0.0.1/cb").success).toBe(true); }); + + it("rejects redirect URIs with a fragment component", () => { + expect( + SafeUrlSchema.safeParse("https://example.com/cb#token").success, + ).toBe(false); + expect(SafeUrlSchema.safeParse("https://example.com/cb#").success).toBe( + false, + ); + expect(SafeUrlSchema.safeParse("https://example.com/cb").success).toBe( + true, + ); + }); }); diff --git a/packages/core/src/utils/url.ts b/packages/core/src/utils/url.ts index da85d4b4fc..13b9a74c74 100644 --- a/packages/core/src/utils/url.ts +++ b/packages/core/src/utils/url.ts @@ -60,9 +60,12 @@ export const DANGEROUS_URL_SCHEMES = ["javascript:", "data:", "vbscript:"]; * execution schemes without rejecting relative paths or mobile deep links. */ export function isSafeUrlScheme(value: string): boolean { - if (!URL.canParse(value)) { + let parsed: URL; + try { + parsed = new URL(value); + } catch { // Relative URLs carry no scheme to abuse. return true; } - return !DANGEROUS_URL_SCHEMES.includes(new URL(value).protocol); + return !DANGEROUS_URL_SCHEMES.includes(parsed.protocol); }