From 51e531667936ed00550ecb24aa93227719244541 Mon Sep 17 00:00:00 2001 From: Dominic Couture Date: Mon, 17 Aug 2026 10:33:24 +0200 Subject: [PATCH] fix(backend): Remove unnecessary custom cookie parsing Fix `Cookie` header parsing in `authenticateRequest()` to follow RFC 6265. The header is now split into name/value pairs before each value is percent-decoded, instead of the whole header being decoded up front. Previously, a cookie whose value contained an encoded `;` or `=` (for example `pref=a%3Bb%3Dc`, which is what most frameworks emit when a value is set from user input) was split into several separate cookies, so Clerk could observe cookies that were not present in the request. A value containing a malformed percent-sequence such as `%C0` also caused the request to throw a `URIError`. Legitimate cookie values are unaffected: single-encoded values decode to the same result as before. Also guard against a non-string `iss` claim when comparing a session token against the instance's frontend API. A `__session` cookie holding a JWT whose payload had a numeric `iss` previously threw a `TypeError` while building the authenticate context, before any signature verification ran. --- .changeset/fresh-parrots-decide.md | 6 +++ .../__tests__/authenticateContext.test.ts | 18 +++++++++ .../src/tokens/__tests__/clerkRequest.test.ts | 40 +++++++++++++++++-- .../backend/src/tokens/authenticateContext.ts | 5 +++ packages/backend/src/tokens/clerkRequest.ts | 9 ++--- 5 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 .changeset/fresh-parrots-decide.md diff --git a/.changeset/fresh-parrots-decide.md b/.changeset/fresh-parrots-decide.md new file mode 100644 index 00000000000..ed514789580 --- /dev/null +++ b/.changeset/fresh-parrots-decide.md @@ -0,0 +1,6 @@ +--- +'@clerk/backend': patch +--- + +Fix `Cookie` header parsing in `authenticateRequest()` to follow RFC 6265 and guard against a non-string `iss`. Legitimate cookies and JWTs are unaffected. + diff --git a/packages/backend/src/tokens/__tests__/authenticateContext.test.ts b/packages/backend/src/tokens/__tests__/authenticateContext.test.ts index 5df5e550ba8..0ae390a1f79 100644 --- a/packages/backend/src/tokens/__tests__/authenticateContext.test.ts +++ b/packages/backend/src/tokens/__tests__/authenticateContext.test.ts @@ -212,6 +212,24 @@ describe('AuthenticateContext', () => { expect(context.clientUat.toString()).toBe('0'); }); }); + + describe('malformed un-suffixed session token', () => { + it('falls back to suffixed cookies when the token has a non-string issuer', async () => { + const headers = new Headers({ + cookie: createCookieHeader({ + __session: createJwt({ payload: { iss: 123 as unknown as string } }), + __client_uat_MqCvchyS: suffixedClientUat, + __session_MqCvchyS: suffixedSession, + }), + }); + const clerkRequest = createClerkRequest(new Request('http://example.com', { headers })); + + const context = await createAuthenticateContext(clerkRequest, { publishableKey: pkLive }); + + expect(context.usesSuffixedCookies()).toBe(true); + expect(context.sessionTokenInCookie).toBe(suffixedSession); + }); + }); }); describe('relative proxyUrl resolution', () => { diff --git a/packages/backend/src/tokens/__tests__/clerkRequest.test.ts b/packages/backend/src/tokens/__tests__/clerkRequest.test.ts index 1b75b88cd44..da5572ed0f7 100644 --- a/packages/backend/src/tokens/__tests__/clerkRequest.test.ts +++ b/packages/backend/src/tokens/__tests__/clerkRequest.test.ts @@ -89,12 +89,46 @@ describe('createClerkRequest', () => { expect(req.cookies.get('foo')).toBe('bar'); }); - it('should parse and return cookies with special characters', () => { + it('should percent-decode cookie values', () => { + const req = createClerkRequest( + new Request('http://localhost:3000', { headers: new Headers({ cookie: 'foo=hello%20world' }) }), + ); + expect(req.cookies.get('foo')).toBe('hello world'); + }); + + it('should not treat encoded delimiters inside a value as cookie separators', () => { const req = createClerkRequest( new Request('http://localhost:3000', { headers: new Headers({ cookie: 'foo=%20bar%3B%20baz%3Dqux' }) }), ); - expect(req.cookies.get('foo')).toBe('bar'); - expect(req.cookies.get('baz')).toBe('qux'); + expect(req.cookies.get('foo')).toBe(' bar; baz=qux'); + expect(req.cookies.get('baz')).toBeUndefined(); + }); + + it('should not let an encoded value forge a session cookie', () => { + const req = createClerkRequest( + new Request('http://localhost:3000', { + headers: new Headers({ cookie: 'sid=x%3B__session%3Dforged%3B__client_uat%3D1700000000' }), + }), + ); + expect(req.cookies.get('sid')).toBe('x;__session=forged;__client_uat=1700000000'); + expect(req.cookies.get('__session')).toBeUndefined(); + expect(req.cookies.get('__client_uat')).toBeUndefined(); + }); + + it('should keep the first value when a cookie name is repeated', () => { + const req = createClerkRequest( + new Request('http://localhost:3000', { headers: new Headers({ cookie: '__session=first; __session=second' }) }), + ); + expect(req.cookies.get('__session')).toBe('first'); + }); + + it('should not throw on malformed percent-sequences', () => { + const req = createClerkRequest( + new Request('http://localhost:3000', { headers: new Headers({ cookie: 'foo=%C0; bar=100%D0; baz=ok' }) }), + ); + expect(req.cookies.get('foo')).toBe('%C0'); + expect(req.cookies.get('bar')).toBe('100%D0'); + expect(req.cookies.get('baz')).toBe('ok'); }); it('should parse and return cookies even if no cookie header exists', () => { diff --git a/packages/backend/src/tokens/authenticateContext.ts b/packages/backend/src/tokens/authenticateContext.ts index aaf1d8f7e04..427b49ae7b3 100644 --- a/packages/backend/src/tokens/authenticateContext.ts +++ b/packages/backend/src/tokens/authenticateContext.ts @@ -386,6 +386,11 @@ class AuthenticateContext implements AuthenticateContext { if (errors) { return false; } + // `decodeJwt` only JSON-parses the payload, so `iss` is an unvalidated attacker-supplied + // value here — this runs before any signature verification. + if (typeof data.payload.iss !== 'string') { + return false; + } const tokenIssuer = data.payload.iss.replace(/https?:\/\//gi, ''); // Use original frontend API for token validation since tokens are issued by the actual Clerk API, not proxy return this.originalFrontendApi === tokenIssuer; diff --git a/packages/backend/src/tokens/clerkRequest.ts b/packages/backend/src/tokens/clerkRequest.ts index 8b02266c643..5ec9ee63908 100644 --- a/packages/backend/src/tokens/clerkRequest.ts +++ b/packages/backend/src/tokens/clerkRequest.ts @@ -94,13 +94,12 @@ class ClerkRequest extends Request { } private parseCookies(req: Request) { - const cookiesRecord = parse(this.decodeCookieValue(req.headers.get('cookie') || '')); + // Hand the raw header to `parse`, which splits on the RFC 6265 delimiters before + // percent-decoding each value. Decoding the header first would promote an encoded + // `;`/`=` inside one value into a delimiter, forging additional cookies. + const cookiesRecord = parse(req.headers.get('cookie') || ''); return new Map(Object.entries(cookiesRecord)); } - - private decodeCookieValue(str: string) { - return str ? str.replace(/(%[0-9A-Z]{2})+/g, decodeURIComponent) : str; - } } export const createClerkRequest = (...args: ConstructorParameters): ClerkRequest => {