fix: open redirect

e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts

@@ -0,0 +1,110 @@
+import { expect, test } from '@playwright/test'
+
+test.describe('Open redirect prevention', () => {
+  test.describe('CRLF injection attacks', () => {
+    test('should not redirect to external site via CR injection (%0d)', async ({
+      page,
+      baseURL,
+    }) => {
+      // This URL attempts to exploit CRLF injection to redirect to google.com
+      // %0d = \r (carriage return)
+      // Without the fix, /\r/google.com/ would be interpreted as //google.com/
+      // which browsers resolve as a protocol-relative URL to http://google.com/
+      await page.goto('/%0d/google.com/')
+      await page.waitForLoadState('networkidle')
+
+      // Should stay on the same origin - the path should be treated as a local path
+      // not as an external redirect
+      expect(page.url().startsWith(baseURL!)).toBe(true)
+      // The origin should be localhost, not google.com
+      const url = new URL(page.url())
+      expect(url.origin).toBe(new URL(baseURL!).origin)
+    })
+
+    test('should not redirect to external site via LF injection (%0a)', async ({
+      page,
+      baseURL,
+    }) => {
+      // %0a = \n (line feed)
+      await page.goto('/%0a/evil.com/')
+      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)
+    })
+
+    test('should not redirect to external site via CRLF injection (%0d%0a)', async ({
+      page,
+      baseURL,
+    }) => {
+      // Combined CRLF injection attempt
+      await page.goto('/%0d%0a/attacker.com/')
+      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)
+    })
+
+    test('should not redirect to external site via multiple control chars', async ({
+      page,
+      baseURL,
+    }) => {
+      // Multiple CR/LF characters
+      await page.goto('/%0d%0d%0a/malicious.com/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)
+    })
+  })
+
+  test.describe('Protocol-relative URL prevention', () => {
+    test('should handle paths that could be misinterpreted as protocol-relative URLs', async ({
+      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
+      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\/?$/)
+    })
+  })
+
+  test.describe('Normal navigation still works', () => {
+    test('should navigate to normal paths correctly', async ({
+      page,
+      baseURL,
+    }) => {
+      await page.goto('/posts')
+      await page.waitForLoadState('networkidle')
+
+      expect(page.url()).toBe(`${baseURL}/posts`)
+    })
+
+    test('should handle URL-encoded characters in normal paths', async ({
+      page,
+      baseURL,
+    }) => {
+      // Normal URL encoding should still work
+      await page.goto('/params/single/hello%20world')
+      await page.waitForLoadState('networkidle')
+
+      expect(page.url().startsWith(baseURL!)).toBe(true)
+    })
+  })
+})

e2e/react-start/basic/tests/open-redirect-prevention.spec.ts

@@ -0,0 +1,117 @@
+import { expect } from '@playwright/test'
+import { test } from '@tanstack/router-e2e-utils'
+
+test.use({
+  whitelistErrors: [
+    /Failed to load resource: the server responded with a status of 404/,
+  ],
+})
+
+test.describe('Open redirect prevention', () => {
+  test.describe('CRLF injection attacks', () => {
+    test('should not redirect to external site via CR injection (%0d)', async ({
+      page,
+      baseURL,
+    }) => {
+      // This URL attempts to exploit CRLF injection to redirect to google.com
+      // %0d = \r (carriage return)
+      // Without the fix, /\r/google.com/ would be interpreted as //google.com/
+      // which browsers resolve as a protocol-relative URL to http://google.com/
+      await page.goto('/%0d/google.com/')
+      await page.waitForLoadState('networkidle')
+
+      // Should stay on the same origin - the path should be treated as a local path
+      // not as an external redirect
+      expect(page.url().startsWith(baseURL!)).toBe(true)
+      // The origin should be localhost, not google.com
+      const url = new URL(page.url())
+      expect(url.origin).toBe(new URL(baseURL!).origin)
+    })
+
+    test('should not redirect to external site via LF injection (%0a)', async ({
+      page,
+      baseURL,
+    }) => {
+      // %0a = \n (line feed)
+      await page.goto('/%0a/evil.com/')
+      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)
+    })
+
+    test('should not redirect to external site via CRLF injection (%0d%0a)', async ({
+      page,
+      baseURL,
+    }) => {
+      // Combined CRLF injection attempt
+      await page.goto('/%0d%0a/attacker.com/')
+      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)
+    })
+
+    test('should not redirect to external site via multiple control chars', async ({
+      page,
+      baseURL,
+    }) => {
+      // Multiple CR/LF characters
+      await page.goto('/%0d%0d%0a/malicious.com/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)
+    })
+  })
+
+  test.describe('Protocol-relative URL prevention', () => {
+    test('should handle paths that could be misinterpreted as protocol-relative URLs', async ({
+      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
+      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\/?$/)
+    })
+  })
+
+  test.describe('Normal navigation still works', () => {
+    test('should navigate to normal paths correctly', async ({
+      page,
+      baseURL,
+    }) => {
+      await page.goto('/posts')
+      await page.waitForLoadState('networkidle')
+
+      expect(page.url()).toBe(`${baseURL}/posts`)
+    })
+
+    test('should handle URL-encoded characters in normal paths', async ({
+      page,
+      baseURL,
+    }) => {
+      // Normal URL encoding should still work
+      await page.goto('/posts/1')
+      await page.waitForLoadState('networkidle')
+
+      expect(page.url()).toBe(`${baseURL}/posts/1`)
+    })
+  })
+})

packages/history/src/index.ts

@@ -623,31 +623,54 @@ export function createMemoryHistory(
   })
 }
 
+/**
+ * Sanitize a path to prevent open redirect vulnerabilities.
+ * Removes control characters and collapses leading double slashes.
+ */
+function sanitizePath(path: 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
+  let sanitized = path.replace(/[\x00-\x1f\x7f]/g, '')
+
+  // Prevent open redirect via protocol-relative URLs (e.g. "//evil.com")
+  // Collapse leading double slashes to a single slash
+  if (sanitized.startsWith('//')) {
+    sanitized = '/' + sanitized.replace(/^\/+/, '')
+  }
+
+  return sanitized
+}
+
 export function parseHref(
   href: string,
   state: ParsedHistoryState | undefined,
 ): HistoryLocation {
-  const hashIndex = href.indexOf('#')
-  const searchIndex = href.indexOf('?')
+  const sanitizedHref = sanitizePath(href)
+  const hashIndex = sanitizedHref.indexOf('#')
+  const searchIndex = sanitizedHref.indexOf('?')
 
   const addedKey = createRandomKey()
 
   return {
-    href,
-    pathname: href.substring(
+    href: sanitizedHref,
+    pathname: sanitizedHref.substring(
       0,
       hashIndex > 0
         ? searchIndex > 0
           ? Math.min(hashIndex, searchIndex)
           : hashIndex
         : searchIndex > 0
           ? searchIndex
-          : href.length,
+          : sanitizedHref.length,
     ),
-    hash: hashIndex > -1 ? href.substring(hashIndex) : '',
+    hash: hashIndex > -1 ? sanitizedHref.substring(hashIndex) : '',
     search:
       searchIndex > -1
-        ? href.slice(searchIndex, hashIndex === -1 ? undefined : hashIndex)
+        ? sanitizedHref.slice(
+            searchIndex,
+            hashIndex === -1 ? undefined : hashIndex,
+          )
         : '',
     state: state || { [stateIndexKey]: 0, key: addedKey, __TSR_key: addedKey },
   }

packages/history/tests/parseHref.test.ts

@@ -3,9 +3,54 @@ import { parseHref } from '../src'
 
 describe('parseHref', () => {
   test('works', () => {
-    const parsed = parseHref('/foo?bar=baz#qux', {})
+    const parsed = parseHref('/foo?bar=baz#qux', {} as any)
     expect(parsed.pathname).toEqual('/foo')
     expect(parsed.search).toEqual('?bar=baz')
     expect(parsed.hash).toEqual('#qux')
   })
+
+  describe('open redirect prevention', () => {
+    test('strips CR characters to prevent open redirect', () => {
+      // If \r (CR) is in the href, it should be stripped
+      const parsed = parseHref('/\r/google.com/', undefined)
+      expect(parsed.href).toBe('/google.com/')
+      expect(parsed.pathname).toBe('/google.com/')
+      expect(parsed.href).not.toMatch(/^\/\//)
+    })
+
+    test('strips LF characters to prevent open redirect', () => {
+      const parsed = parseHref('/\n/evil.com/', undefined)
+      expect(parsed.href).toBe('/evil.com/')
+      expect(parsed.pathname).toBe('/evil.com/')
+      expect(parsed.href).not.toMatch(/^\/\//)
+    })
+
+    test('strips CRLF characters to prevent open redirect', () => {
+      const parsed = parseHref('/\r\n/evil.com/', undefined)
+      expect(parsed.href).toBe('/evil.com/')
+      expect(parsed.pathname).toBe('/evil.com/')
+      expect(parsed.href).not.toMatch(/^\/\//)
+    })
+
+    test('collapses leading double slashes to prevent protocol-relative URLs', () => {
+      const parsed = parseHref('//evil.com/path', undefined)
+      expect(parsed.href).toBe('/evil.com/path')
+      expect(parsed.pathname).toBe('/evil.com/path')
+    })
+
+    test('sanitized href resolves safely to same origin', () => {
+      const parsed = parseHref('/\r/evil.com/', undefined)
+      const url = new URL(parsed.href, 'http://localhost:3000')
+      expect(url.origin).toBe('http://localhost:3000')
+      expect(url.pathname).toBe('/evil.com/')
+    })
+
+    test('normal paths remain unchanged', () => {
+      const parsed = parseHref('/users/profile?id=1#section', undefined)
+      expect(parsed.href).toBe('/users/profile?id=1#section')
+      expect(parsed.pathname).toBe('/users/profile')
+      expect(parsed.search).toBe('?id=1')
+      expect(parsed.hash).toBe('#section')
+    })
+  })
 })

packages/router-core/src/utils.ts

@@ -489,19 +489,33 @@ export function findLast<T>(
   return undefined
 }
 
+/**
+ * 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".
+ */
+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, '')
+}
+
 function decodeSegment(segment: string): string {
+  let decoded: string
   try {
-    return decodeURI(segment)
+    decoded = decodeURI(segment)
   } catch {
     // if the decoding fails, try to decode the various parts leaving the malformed tags in place
-    return segment.replaceAll(/%[0-9A-F]{2}/gi, (match) => {
+    decoded = segment.replaceAll(/%[0-9A-F]{2}/gi, (match) => {
       try {
         return decodeURI(match)
       } catch {
         return match
       }
     })
   }
+  return sanitizePathSegment(decoded)
 }
 
 export function decodePath(path: string, decodeIgnore?: Array<string>): string {
@@ -516,5 +530,14 @@ export function decodePath(path: string, decodeIgnore?: Array<string>): string {
     result += decodeSegment(path.slice(cursor, match.index)) + match[0]
     cursor = re.lastIndex
   }
-  return result + decodeSegment(cursor ? path.slice(cursor) : path)
+  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
+  if (result.startsWith('//')) {
+    result = '/' + result.replace(/^\/+/, '')
+  }
+
+  return result
 }