diff --git a/.changeset/dark-bars-wink.md b/.changeset/dark-bars-wink.md new file mode 100644 index 00000000000..de3ad38653a --- /dev/null +++ b/.changeset/dark-bars-wink.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +Add path traversal protections in `joinPaths` diff --git a/packages/backend/src/util/__tests__/path.test.ts b/packages/backend/src/util/__tests__/path.test.ts index 1c92cc6b7d7..470092ed836 100644 --- a/packages/backend/src/util/__tests__/path.test.ts +++ b/packages/backend/src/util/__tests__/path.test.ts @@ -38,4 +38,54 @@ describe('utils.joinPaths(...args)', () => { it('handles no input', () => { expect(joinPaths()).toBe(''); }); + + it('accepts "." and ".." within a segment (not entire segment)', () => { + // Dot not as an isolated path segment + expect(joinPaths('foo.bar', 'baz')).toBe('foo.bar/baz'); + expect(joinPaths('foo..bar', 'baz')).toBe('foo..bar/baz'); + expect(joinPaths('foo.', 'bar.')).toBe('foo./bar.'); + expect(joinPaths('foo..', '..bar')).toBe('foo../..bar'); + expect(joinPaths('foo..baz')).toBe('foo..baz'); + expect(joinPaths('fo.o', 'ba..z')).toBe('fo.o/ba..z'); + }); + + it('accepts "." and ".." inside query parameter or as value', () => { + // . and .. as values in query string should not be considered dot segments + expect(joinPaths('/api', 'users?filter=..')).toBe('/api/users?filter=..'); + expect(joinPaths('/api', 'users?filter=.')).toBe('/api/users?filter=.'); + expect(joinPaths('/v1', 'search?q=foo.bar..baz')).toBe('/v1/search?q=foo.bar..baz'); + // . and .. within querystring, fragment, or a value + expect(joinPaths('/foo', '?bar=..&baz=.')).toBe('/foo/?bar=..&baz=.'); + expect(joinPaths('/foo', '#frag..ment')).toBe('/foo/#frag..ment'); + }); + + it('rejects literal ".." segments', () => { + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '../../../users')).toThrow(); + expect(() => joinPaths('/sessions', '..')).toThrow(); + }); + + it('rejects "." segments', () => { + expect(() => joinPaths('foo/./bar')).toThrow(); + expect(() => joinPaths('foo', '.', 'bar')).toThrow(); + expect(() => joinPaths('foo', './', 'bar')).toThrow(); + }); + + it('rejects percent-encoded dot segments', () => { + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '%2e%2e/users')).toThrow(); + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '%2E%2E/users')).toThrow(); + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '.%2E/users')).toThrow(); + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '%2e%2e%2fusers')).toThrow(); + expect(() => joinPaths('/sessions', 'sess_abc', 'tokens', '%2e%2e%252fusers')).toThrow(); + expect(() => joinPaths('foo', '%2e', 'bar')).toThrow(); + }); + + it('rejects too many layers of encoding', () => { + expect(() => joinPaths('foo', '%2525252525252525252525252541')).toThrow(); + }); + + it('allows legitimate URLs and ID-like segments', () => { + expect(joinPaths('https://api.clerk.com', 'v1', '/sessions/sess_abc/tokens/supabase')).toBe( + 'https://api.clerk.com/v1/sessions/sess_abc/tokens/supabase', + ); + }); }); diff --git a/packages/backend/src/util/path.ts b/packages/backend/src/util/path.ts index 3e191aa6436..f6523a151a4 100644 --- a/packages/backend/src/util/path.ts +++ b/packages/backend/src/util/path.ts @@ -1,11 +1,43 @@ const SEPARATOR = '/'; const MULTIPLE_SEPARATOR_REGEX = new RegExp('(? p === '.' || p === '..')) { + return true; + } + if (i === MAX_DECODES) { + throw new Error(`joinPaths: too many layers of encoding in ${segment}`); + } + try { + const next = decodeURIComponent(candidate); + if (next === candidate) { + break; + } // stable — no more encoding + candidate = next; + } catch { + break; + } + } + return false; +} + export function joinPaths(...args: PathString[]): string { - return args + const result = args .filter(p => p) .join(SEPARATOR) .replace(MULTIPLE_SEPARATOR_REGEX, SEPARATOR); + + for (const segment of result.split(SEPARATOR)) { + if (isDotSegment(segment)) { + throw new Error(`joinPaths: "." and ".." path segments are not allowed (received "${result}")`); + } + } + + return result; }