diff --git a/.changeset/fix-decode-path-preserve-unsafe.md b/.changeset/fix-decode-path-preserve-unsafe.md new file mode 100644 index 0000000000..a68ae9e3e1 --- /dev/null +++ b/.changeset/fix-decode-path-preserve-unsafe.md @@ -0,0 +1,9 @@ +--- +'@tanstack/router-core': patch +--- + +fix(router-core): re-encode URL-unsafe characters in `sanitizePathSegment` to prevent infinite redirect loops + +`sanitizePathSegment` now re-encodes characters in the WHATWG URL "path percent-encode set" (`<`, `>`, `"`, `` ` ``, `{`, `}`) and ASCII control characters back to their percent-encoded form, instead of stripping control characters. This prevents mismatches between the original URL and the router's internal representation that previously caused infinite 307 redirect loops on paths containing these characters (e.g. `/%7B%7Btemplate%7D%7D`). + +Fixes #7587. diff --git a/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts b/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts index 3ad83fb455..2f0fe256cd 100644 --- a/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts +++ b/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts @@ -69,10 +69,7 @@ test.describe('Open redirect prevention', () => { page, baseURL, }) => { - // When control characters are stripped from paths like /%0d/evil.com/ - // the result could be //evil.com/ which is a protocol-relative URL - // Our fix collapses these to /evil.com/ to prevent external redirects - // This is already tested above, but we verify the collapsed path works + // %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/ await page.goto('/%0d/test-path/') await page.waitForLoadState('networkidle') @@ -80,8 +77,6 @@ test.describe('Open redirect prevention', () => { expect(page.url().startsWith(baseURL!)).toBe(true) const url = new URL(page.url()) expect(url.origin).toBe(new URL(baseURL!).origin) - // Path should be collapsed to /test-path (not //test-path/) - expect(url.pathname).toMatch(/^\/test-path\/?$/) }) }) diff --git a/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts b/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts index e497109889..bc88c33605 100644 --- a/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts +++ b/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts @@ -77,22 +77,14 @@ test.describe('Open redirect prevention', () => { page, baseURL, }) => { - // When control characters are stripped from paths like /%0d/evil.com/ - // the result could be //evil.com/ which is a protocol-relative URL - // Our fix collapses these to /evil.com/ to prevent external redirects - // This is already tested above, but we verify the collapsed path works - const res = await page.goto('/%0d/test-path/') + // %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/ + await page.goto('/%0d/test-path/') await page.waitForLoadState('networkidle') // Should stay on the same origin expect(page.url().startsWith(baseURL!)).toBe(true) const url = new URL(page.url()) expect(url.origin).toBe(new URL(baseURL!).origin) - // Path should be collapsed to /test-path (not //test-path/) - expect(url.pathname).toMatch(/^\/test-path\/?$/) - if (!isSpaMode) { - expect(res?.request().redirectedFrom()?.url()).not.toBe(undefined) - } }) }) diff --git a/e2e/react-start/basic/tests/special-characters.spec.ts b/e2e/react-start/basic/tests/special-characters.spec.ts index e88280e2e1..7cf50c39c8 100644 --- a/e2e/react-start/basic/tests/special-characters.spec.ts +++ b/e2e/react-start/basic/tests/special-characters.spec.ts @@ -64,6 +64,22 @@ test.describe('Unicode route rendering', () => { expect(param).toBe('대|') }) + + test('should not cause redirect loop for curly braces in path params', async ({ + page, + baseURL, + }) => { + // %7B/%7D (curly braces) must not trigger infinite 307 redirects + const res = await page.goto('/specialChars/%7B%7Bapp_name%7D%7D') + await page.waitForLoadState('load') + + // Should not redirect — URL stays encoded + expect(page.url()).toBe(`${baseURL}/specialChars/%7B%7Bapp_name%7D%7D`) + expect(res!.status()).toBe(200) + + const param = await page.getByTestId('special-param').textContent() + expect(param).toBe('{{app_name}}') + }) }) test.describe('Special characters in search params', () => { diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index f017215b5c..91011bfa7f 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -523,15 +523,26 @@ export function findLast( } /** - * Remove control characters that can cause open redirect vulnerabilities. - * Characters like \r (CR) and \n (LF) can trick URL parsers into interpreting - * paths like "/\r/evil.com" as "http://evil.com". + * Re-encode characters that are unsafe in URL paths. + * Includes ASCII control characters (0x00-0x1F, 0x7F) and a subset of the + * WHATWG URL "path percent-encode set" (", <, >, `, {, }). + * + * Space (0x20) is intentionally excluded — decodeURI decodes %20 to space + * and the router stores decoded spaces in location.pathname. The existing + * encodePathLikeUrl already handles re-encoding spaces for outgoing URLs. + * + * These characters are decoded by decodeURI but must remain percent-encoded + * in paths to match how upstream layers (CDNs, edge middleware, browsers) + * interpret the URL, preventing infinite redirect loops and path mismatches. */ +// eslint-disable-next-line no-control-regex +const PATH_UNSAFE_RE = /[\x00-\x1f\x7f"<>`{}]/g + function sanitizePathSegment(segment: string): string { - // Remove ASCII control characters (0x00-0x1F) and DEL (0x7F) - // These include CR (\r = 0x0D), LF (\n = 0x0A), and other potentially dangerous characters - // eslint-disable-next-line no-control-regex - return segment.replace(/[\x00-\x1f\x7f]/g, '') + return segment.replace( + PATH_UNSAFE_RE, + (ch) => '%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'), + ) } function decodeSegment(segment: string): string { @@ -644,8 +655,9 @@ export function decodePath(path: string) { result = result + decodeSegment(cursor ? path.slice(cursor) : path) // Prevent open redirect via protocol-relative URLs (e.g. "//evil.com") - // After sanitizing control characters, paths like "/\r/evil.com" become "//evil.com" - // Collapse leading double slashes to a single slash + // This is defense-in-depth: since control characters are no longer decoded, + // paths like "/%0d/evil.com" can no longer become "//evil.com". But we keep + // this check to guard against other edge cases. let handledProtocolRelativeURL = false if (result.startsWith('//')) { handledProtocolRelativeURL = true diff --git a/packages/router-core/tests/utils.test.ts b/packages/router-core/tests/utils.test.ts index 95874f230e..fff911df37 100644 --- a/packages/router-core/tests/utils.test.ts +++ b/packages/router-core/tests/utils.test.ts @@ -547,46 +547,45 @@ describe('decodePath', () => { }) describe('open redirect prevention', () => { - it('should strip CR (%0d) to prevent open redirect', () => { - // /%0d/google.com/ decodes to /\r/google.com/ which becomes //google.com/ - // This must be sanitized to prevent protocol-relative URL interpretation + it('should keep CR (%0d) encoded to prevent open redirect', () => { + // %0d stays encoded — no decoding, no stripping, no path mismatch + // Output is uppercase hex per RFC 3986 const result = decodePath('/%0d/google.com/') - expect(result.path).toBe('/google.com/') + expect(result.path).toBe('/%0D/google.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip LF (%0a) to prevent open redirect', () => { + it('should keep LF (%0a) encoded to prevent open redirect', () => { const result = decodePath('/%0a/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0A/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip CRLF (%0d%0a) to prevent open redirect', () => { + it('should keep CRLF (%0d%0a) encoded to prevent open redirect', () => { const result = decodePath('/%0d%0a/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0D%0A/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip multiple control characters to prevent open redirect', () => { + it('should keep multiple control characters encoded', () => { const result = decodePath('/%0d%0d%0d/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0D%0D%0D/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip null bytes and other control characters', () => { + it('should keep null bytes encoded', () => { const result = decodePath('/%00/test/') - expect(result.path).toBe('/test/') - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.path).toBe('/%00/test/') + expect(result.handledProtocolRelativeURL).toBe(false) }) it('should collapse leading double slashes to prevent protocol-relative URLs', () => { - // After stripping control chars, ensure we don't end up with //evil.com - const result = decodePath('/%0d%0a/evil.com/path') - // Should resolve to localhost, not evil.com + // Direct // input should still be collapsed as defense-in-depth + const result = decodePath('//evil.com/path') const url = new URL(result.path, 'http://localhost:3000') expect(url.origin).toBe('http://localhost:3000') expect(result.handledProtocolRelativeURL).toBe(true) @@ -608,6 +607,34 @@ describe('decodePath', () => { expect(result.handledProtocolRelativeURL).toBe(true) }) }) + + describe('WHATWG path percent-encode set preserved', () => { + it('should keep curly braces encoded', () => { + const result = decodePath('/%7B%7Bapp_name%7D%7D/Makefile') + expect(result.path).toBe('/%7B%7Bapp_name%7D%7D/Makefile') + }) + + it('should keep angle brackets encoded', () => { + expect(decodePath('/%3Ctest%3E').path).toBe('/%3Ctest%3E') + }) + + it('should keep double quotes encoded', () => { + expect(decodePath('/foo%22bar').path).toBe('/foo%22bar') + }) + + it('should keep backticks encoded', () => { + expect(decodePath('/back%60tick').path).toBe('/back%60tick') + }) + + it('should decode space (handled by encodePathLikeUrl for outgoing URLs)', () => { + expect(decodePath('/file%20name').path).toBe('/file name') + }) + + it('should still decode safe characters', () => { + // Regular letters/unicode should still be decoded + expect(decodePath('/%D1%88%D0%B5%D0%BB%D0%BB%D1%8B').path).toBe('/шеллы') + }) + }) }) /**