Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion e2e/react-start/basic/tests/open-redirect-prevention.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@playwright/test'
import { test } from '@tanstack/router-e2e-utils'
import { isSpaMode } from './utils/isSpaMode'

test.use({
whitelistErrors: [
Expand Down Expand Up @@ -80,7 +81,7 @@ test.describe('Open redirect prevention', () => {
// 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/')
const res = await page.goto('/%0d/test-path/')
await page.waitForLoadState('networkidle')

// Should stay on the same origin
Expand All @@ -89,6 +90,9 @@ test.describe('Open redirect prevention', () => {
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)
}
})
})

Expand Down
10 changes: 5 additions & 5 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1268,14 +1268,14 @@ export class RouterCore<
return {
href: pathname + searchStr + hash,
publicHref: href,
pathname: decodePath(pathname),
pathname: decodePath(pathname).path,
external: false,
searchStr,
search: replaceEqualDeep(
previousLocation?.search,
parsedSearch,
) as any,
hash: decodePath(hash.slice(1)),
hash: decodePath(hash.slice(1)).path,
state: replaceEqualDeep(previousLocation?.state, state),
}
}
Expand All @@ -1297,11 +1297,11 @@ export class RouterCore<
return {
href: fullPath,
publicHref: href,
pathname: decodePath(url.pathname),
pathname: decodePath(url.pathname).path,
external: !!this.rewrite && url.origin !== this.origin,
searchStr,
search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any,
hash: decodePath(url.hash.slice(1)),
hash: decodePath(url.hash.slice(1)).path,
state: replaceEqualDeep(previousLocation?.state, state),
}
}
Expand Down Expand Up @@ -1879,7 +1879,7 @@ export class RouterCore<
decoder: this.pathParamsDecoder,
server: this.isServer,
}).interpolatedPath,
)
).path

// Resolve the next search
let nextSearch = fromSearch
Expand Down
2 changes: 1 addition & 1 deletion packages/router-core/src/ssr/createRequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function createRequestHandler<TRouter extends AnyRouter>({
})

// normalizing and sanitizing the pathname here for server, so we always deal with the same format during SSR.
const url = getNormalizedURL(request.url, 'http://localhost')
const { url } = getNormalizedURL(request.url, 'http://localhost')
const origin = getOrigin(request)
const href = url.href.replace(url.origin, '')

Expand Down
9 changes: 7 additions & 2 deletions packages/router-core/src/ssr/ssr-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,13 +398,18 @@ export function getNormalizedURL(url: string | URL, base?: string | URL) {
if (typeof url === 'string') url = url.replace('\\', '%5C')

const rawUrl = new URL(url, base)
const decodedPathname = decodePath(rawUrl.pathname)
const { path: decodedPathname, handledProtocolRelativeURL } = decodePath(
rawUrl.pathname,
)
const searchParams = new URLSearchParams(rawUrl.search)
const normalizedHref =
decodedPathname +
(searchParams.size > 0 ? '?' : '') +
searchParams.toString() +
rawUrl.hash

return new URL(normalizedHref, rawUrl.origin)
return {
url: new URL(normalizedHref, rawUrl.origin),
handledProtocolRelativeURL,
}
}
14 changes: 7 additions & 7 deletions packages/router-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,21 +593,19 @@ export function escapeHtml(str: string): string {
return str.replace(HTML_ESCAPE_REGEX, (match) => HTML_ESCAPE_LOOKUP[match]!)
}

export function decodePath(path: string, decodeIgnore?: Array<string>): string {
if (!path) return path
export function decodePath(path: string) {
if (!path) return { path, handledProtocolRelativeURL: false }

// Fast path: most paths are already decoded and safe.
// Only fall back to the slower scan/regex path when we see a '%' (encoded),
// a backslash (explicitly handled), a control character, or a protocol-relative
// prefix which needs collapsing.
// eslint-disable-next-line no-control-regex
if (!/[%\\\x00-\x1f\x7f]/.test(path) && !path.startsWith('//')) {
return path
return { path, handledProtocolRelativeURL: false }
}

const re = decodeIgnore
? new RegExp(`${decodeIgnore.join('|')}`, 'gi')
: /%25|%5C/gi
const re = /%25|%5C/gi
let cursor = 0
let result = ''
let match
Expand All @@ -620,11 +618,13 @@ export function decodePath(path: string, decodeIgnore?: Array<string>): string {
// 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
let handledProtocolRelativeURL = false
if (result.startsWith('//')) {
handledProtocolRelativeURL = true
result = '/' + result.replace(/^\/+/, '')
}

return result
return { path: result, handledProtocolRelativeURL }
}

/**
Expand Down
14 changes: 7 additions & 7 deletions packages/router-core/tests/getNormalizedURL.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ describe('getNormalizedURL', () => {
const normalizedUrl1 = getNormalizedURL(url1)
const normalizedUrl2 = getNormalizedURL(url2)

expect(normalizedUrl1.pathname).toBe('/%EB%8C%80|/path')
expect(normalizedUrl1.pathname).toBe(normalizedUrl2.pathname)
expect(normalizedUrl1.url.pathname).toBe('/%EB%8C%80|/path')
expect(normalizedUrl1.url.pathname).toBe(normalizedUrl2.url.pathname)
expect(new URL(url1).pathname).not.toBe(new URL(url2).pathname)

expect(normalizedUrl1.search).toBe(`?query=%EB%8C%80%7C`)
expect(normalizedUrl1.search).toBe(normalizedUrl2.search)
expect(normalizedUrl1.url.search).toBe(`?query=%EB%8C%80%7C`)
expect(normalizedUrl1.url.search).toBe(normalizedUrl2.url.search)
expect(new URL(url1).search).not.toBe(new URL(url2).search)
})

Expand Down Expand Up @@ -120,9 +120,9 @@ describe('getNormalizedURL', () => {
'should treat encoded URL specific characters correctly',
({ url, expectedPathName, expectedHash, expectedSearchParams }) => {
const normalizedUrl = getNormalizedURL(url)
expect(normalizedUrl.pathname).toBe(expectedPathName)
expect(normalizedUrl.search).toBe(expectedSearchParams)
expect(normalizedUrl.hash).toBe(expectedHash)
expect(normalizedUrl.url.pathname).toBe(expectedPathName)
expect(normalizedUrl.url.search).toBe(expectedSearchParams)
expect(normalizedUrl.url.hash).toBe(expectedHash)
},
)
})
121 changes: 32 additions & 89 deletions packages/router-core/tests/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -501,116 +501,47 @@ describe('deepEqual', () => {
})

describe('decodePath', () => {
it('should decode a path segment with no ignored items existing', () => {
const itemsToCheck = ['%25', '%5C']
const stringToCheck =
'https://mozilla.org/?x=%D1%88%D0%B5%D0%BB%D0%BB%D1%8B'
const expectedResult = 'https://mozilla.org/?x=шеллы'

const result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)
})

it('should decode a path segment with one ignored character and one ignored item existing', () => {
const itemsToCheck = ['%25']
const stringToCheck =
'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B'
const expectedResult = 'https://mozilla.org/?x=%25ше\\ллы'

const result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)
})

it('should decode a path segment with multiple ignored characters and first ignored item existing', () => {
const itemsToCheck = ['%25', '%5C']
let stringToCheck =
'https://mozilla.org/?x=%25%D1%88%D0%B5%D0%BB%D0%BB%D1%8B'
let expectedResult = 'https://mozilla.org/?x=%25шеллы'

let result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)

stringToCheck = 'https://mozilla.org/?x=%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B'
expectedResult = 'https://mozilla.org/?x=ше%5Cллы'

result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)
})

it('should decode a path segment with multiple ignored characters and other ignored item existing', () => {
const itemsToCheck = ['%25', '%5C']
let stringToCheck =
'https://mozilla.org/?x=%5C%D1%88%D0%B5%D0%BB%D0%BB%D1%8B'
let expectedResult = 'https://mozilla.org/?x=%5Cшеллы'

let result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)

stringToCheck = 'https://mozilla.org/?x=%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B'
expectedResult = 'https://mozilla.org/?x=ше%5Cллы'

result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)
})

it('should decode a path segment with multiple ignored characters and multiple ignored items existing', () => {
const itemsToCheck = ['%25', '%5C']
const stringToCheck =
'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B'
const expectedResult = 'https://mozilla.org/?x=%25ше%5Cллы'

const result = decodePath(stringToCheck, itemsToCheck)

expect(result).toBe(expectedResult)
})

it('should decode a path segment, ignoring `%` and `\\` by default, with multiple ignored items existing', () => {
const stringToCheck =
'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B%2F'
const expectedResult = 'https://mozilla.org/?x=%25ше%5Cллы%2F'

const result = decodePath(stringToCheck)
const result = decodePath(stringToCheck).path

expect(result).toBe(expectedResult)
})

it('should handle malformed percent-encodings gracefully', () => {
const stringToCheck = 'path%ZZ%D1%88test%5C%C3%A9'
// Malformed sequences should remain as-is, valid ones decoded
const result = decodePath(stringToCheck)
const result = decodePath(stringToCheck).path
expect(result).toBe(`path%ZZ%D1%88test%5Cé`)
})

it('should return empty string unchanged', () => {
expect(decodePath('')).toBe('')
expect(decodePath('').path).toBe('')
})

it('should return strings without encoding unchanged', () => {
const stringToCheck = 'plain-text-path'
expect(decodePath(stringToCheck)).toBe(stringToCheck)
expect(decodePath(stringToCheck).path).toBe(stringToCheck)
})

it('should handle consecutive ignored characters', () => {
const stringToCheck = 'test%25%25end'
const expectedResult = 'test%25%25end'
expect(decodePath(stringToCheck)).toBe(expectedResult)
expect(decodePath(stringToCheck).path).toBe(expectedResult)
})

it('should handle multiple ignored items of the same type with varying case', () => {
const stringToCheck = '/params-ps/named/foo%2Fabc/c%2Fh'
const expectedResult = '/params-ps/named/foo%2Fabc/c%2Fh'
expect(decodePath(stringToCheck)).toBe(expectedResult)
expect(decodePath(stringToCheck).path).toBe(expectedResult)

const stringToCheckWithLowerCase = '/params-ps/named/foo%2Fabc/c%5C%2f%5cAh'
const expectedResultWithLowerCase =
'/params-ps/named/foo%2Fabc/c%5C%2f%5cAh'
expect(decodePath(stringToCheckWithLowerCase)).toBe(
expect(decodePath(stringToCheckWithLowerCase).path).toBe(
expectedResultWithLowerCase,
)
})
Expand All @@ -620,49 +551,61 @@ describe('decodePath', () => {
// /%0d/google.com/ decodes to /\r/google.com/ which becomes //google.com/
// This must be sanitized to prevent protocol-relative URL interpretation
const result = decodePath('/%0d/google.com/')
expect(result).toBe('/google.com/')
expect(result).not.toMatch(/^\/\//)
expect(result.path).toBe('/google.com/')
expect(result.path).not.toMatch(/^\/\//)
expect(result.handledProtocolRelativeURL).toBe(true)
})

it('should strip LF (%0a) to prevent open redirect', () => {
const result = decodePath('/%0a/evil.com/')
expect(result).toBe('/evil.com/')
expect(result).not.toMatch(/^\/\//)
expect(result.path).toBe('/evil.com/')
expect(result.path).not.toMatch(/^\/\//)
expect(result.handledProtocolRelativeURL).toBe(true)
})

it('should strip CRLF (%0d%0a) to prevent open redirect', () => {
const result = decodePath('/%0d%0a/evil.com/')
expect(result).toBe('/evil.com/')
expect(result).not.toMatch(/^\/\//)
expect(result.path).toBe('/evil.com/')
expect(result.path).not.toMatch(/^\/\//)
expect(result.handledProtocolRelativeURL).toBe(true)
})

it('should strip multiple control characters to prevent open redirect', () => {
const result = decodePath('/%0d%0d%0d/evil.com/')
expect(result).toBe('/evil.com/')
expect(result).not.toMatch(/^\/\//)
expect(result.path).toBe('/evil.com/')
expect(result.path).not.toMatch(/^\/\//)
expect(result.handledProtocolRelativeURL).toBe(true)
})

it('should strip null bytes and other control characters', () => {
const result = decodePath('/%00/test/')
expect(result).toBe('/test/')
expect(result.path).toBe('/test/')
expect(result.handledProtocolRelativeURL).toBe(true)
})

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
const url = new URL(result, 'http://localhost:3000')
const url = new URL(result.path, 'http://localhost:3000')
expect(url.origin).toBe('http://localhost:3000')
expect(result.handledProtocolRelativeURL).toBe(true)
})

it('should handle normal paths unchanged', () => {
expect(decodePath('/users/profile/')).toBe('/users/profile/')
expect(decodePath('/api/v1/data')).toBe('/api/v1/data')
expect(decodePath('/users/profile/').path).toBe('/users/profile/')
expect(decodePath('/users/profile/').handledProtocolRelativeURL).toBe(
false,
)
expect(decodePath('/api/v1/data').path).toBe('/api/v1/data')
expect(decodePath('/api/v1/data').handledProtocolRelativeURL).toBe(false)
})

it('should handle double slash only input', () => {
// Direct // input should also be collapsed
expect(decodePath('//')).toBe('/')
const result = decodePath('//')
expect(result.path).toBe('/')
expect(result.handledProtocolRelativeURL).toBe(true)
})
})
})
Expand Down
8 changes: 7 additions & 1 deletion packages/start-server-core/src/createStartHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,16 @@ export function createStartHandler<TRegister = Register>(

try {
// normalizing and sanitizing the pathname here for server, so we always deal with the same format during SSR.
const url = getNormalizedURL(request.url)
// during normalization paths like '//posts' are flattened to '/posts'.
// in these cases we would prefer to redirect to the new path
const { url, handledProtocolRelativeURL } = getNormalizedURL(request.url)
const href = url.pathname + url.search + url.hash
const origin = getOrigin(request)

if (handledProtocolRelativeURL) {
return Response.redirect(url, 308)
}

const entries = await getEntries()
const startOptions: AnyStartInstanceOptions =
(await entries.startEntry.startInstance?.getOptions()) ||
Expand Down
Loading