-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(router-core, react-router, solid-router, vue-router): isDangerousProtocol uses customizable allowlist #6542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6124547
0783869
9ca09a4
06a2e15
a134bd1
30fa05d
e39ff4e
4d4c0b9
ffaf0c4
23885c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| import { SAFE_URL_PROTOCOLS, isDangerousProtocol } from './utils' | ||
| import type { NavigateOptions } from './link' | ||
| import type { AnyRouter, RegisteredRouter } from './router' | ||
| import type { ParsedLocation } from './location' | ||
|
|
@@ -124,17 +123,6 @@ export function redirect< | |
| ): Redirect<TRouter, TFrom, TTo, TMaskFrom, TMaskTo> { | ||
| opts.statusCode = opts.statusCode || opts.code || 307 | ||
|
|
||
| // Block dangerous protocols in redirect href | ||
| if ( | ||
| !opts._builtLocation && | ||
| typeof opts.href === 'string' && | ||
| isDangerousProtocol(opts.href) | ||
| ) { | ||
| throw new Error( | ||
| `Redirect blocked: unsafe protocol in href "${opts.href}". Only ${SAFE_URL_PROTOCOLS.join(', ')} protocols are allowed.`, | ||
| ) | ||
| } | ||
|
|
||
|
Comment on lines
-127
to
-137
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to check here, since a |
||
| if ( | ||
| !opts._builtLocation && | ||
| !opts.reloadDocument && | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { createBrowserHistory, parseHref } from '@tanstack/history' | |||||||||||||||
| import { isServer } from '@tanstack/router-core/isServer' | ||||||||||||||||
| import { batch } from './utils/batch' | ||||||||||||||||
| import { | ||||||||||||||||
| DEFAULT_PROTOCOL_ALLOWLIST, | ||||||||||||||||
| createControlledPromise, | ||||||||||||||||
| decodePath, | ||||||||||||||||
| deepEqual, | ||||||||||||||||
|
|
@@ -470,6 +471,15 @@ export interface RouterOptions< | |||||||||||||||
| */ | ||||||||||||||||
| disableGlobalCatchBoundary?: boolean | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * An array of URL protocols to allow in links, redirects, and navigation. | ||||||||||||||||
| * Absolute URLs with protocols not in this list will be rejected. | ||||||||||||||||
| * | ||||||||||||||||
| * @default DEFAULT_PROTOCOL_ALLOWLIST (http:, https:, mailto:, tel:) | ||||||||||||||||
| * @link [API Docs](https://tanstack.com/router/latest/docs/framework/react/api/router/RouterOptionsType#protocolallowlist-property) | ||||||||||||||||
| */ | ||||||||||||||||
| protocolAllowlist?: Array<string> | ||||||||||||||||
|
|
||||||||||||||||
| serializationAdapters?: ReadonlyArray<AnySerializationAdapter> | ||||||||||||||||
| /** | ||||||||||||||||
| * Configures how the router will rewrite the location between the actual href and the internal href of the router. | ||||||||||||||||
|
|
@@ -961,6 +971,7 @@ export class RouterCore< | |||||||||||||||
| resolvePathCache!: LRUCache<string, string> | ||||||||||||||||
| isServer!: boolean | ||||||||||||||||
| pathParamsDecoder?: (encoded: string) => string | ||||||||||||||||
| protocolAllowlist!: Set<string> | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * @deprecated Use the `createRouter` function instead | ||||||||||||||||
|
|
@@ -984,6 +995,8 @@ export class RouterCore< | |||||||||||||||
| notFoundMode: options.notFoundMode ?? 'fuzzy', | ||||||||||||||||
| stringifySearch: options.stringifySearch ?? defaultStringifySearch, | ||||||||||||||||
| parseSearch: options.parseSearch ?? defaultParseSearch, | ||||||||||||||||
| protocolAllowlist: | ||||||||||||||||
| options.protocolAllowlist ?? DEFAULT_PROTOCOL_ALLOWLIST, | ||||||||||||||||
| }) | ||||||||||||||||
|
|
||||||||||||||||
| if (typeof document !== 'undefined') { | ||||||||||||||||
|
|
@@ -1029,6 +1042,8 @@ export class RouterCore< | |||||||||||||||
|
|
||||||||||||||||
| this.isServer = this.options.isServer ?? typeof document === 'undefined' | ||||||||||||||||
|
|
||||||||||||||||
| this.protocolAllowlist = new Set(this.options.protocolAllowlist) | ||||||||||||||||
|
|
||||||||||||||||
|
Comment on lines
+1045
to
+1046
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalize allowlist entries to avoid false blocks for custom protocols. If callers pass ✅ Suggested normalization- this.protocolAllowlist = new Set(this.options.protocolAllowlist)
+ this.protocolAllowlist = new Set(
+ this.options.protocolAllowlist.map((p) => {
+ const normalized = p.toLowerCase()
+ return normalized.endsWith(':') ? normalized : `${normalized}:`
+ }),
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| if (this.options.pathParamsAllowedCharacters) | ||||||||||||||||
| this.pathParamsDecoder = compileDecodeCharMap( | ||||||||||||||||
| this.options.pathParamsAllowedCharacters, | ||||||||||||||||
|
|
@@ -2248,9 +2263,9 @@ export class RouterCore< | |||||||||||||||
| // otherwise use href directly (which may already include basepath) | ||||||||||||||||
| const reloadHref = !hrefIsUrl && publicHref ? publicHref : href | ||||||||||||||||
|
|
||||||||||||||||
| // Block dangerous protocols like javascript:, data:, vbscript: | ||||||||||||||||
| // Block dangerous protocols like javascript:, blob:, data: | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have to change this? |
||||||||||||||||
| // These could execute arbitrary code if passed to window.location | ||||||||||||||||
| if (isDangerousProtocol(reloadHref)) { | ||||||||||||||||
| if (isDangerousProtocol(reloadHref, this.protocolAllowlist)) { | ||||||||||||||||
| if (process.env.NODE_ENV !== 'production') { | ||||||||||||||||
| console.warn( | ||||||||||||||||
| `Blocked navigation to dangerous protocol: ${reloadHref}`, | ||||||||||||||||
|
|
@@ -2669,6 +2684,17 @@ export class RouterCore< | |||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if ( | ||||||||||||||||
| redirect.options.href && | ||||||||||||||||
| !redirect.options._builtLocation && | ||||||||||||||||
| // Check for dangerous protocols before processing the redirect | ||||||||||||||||
| isDangerousProtocol(redirect.options.href, this.protocolAllowlist) | ||||||||||||||||
| ) { | ||||||||||||||||
| throw new Error( | ||||||||||||||||
| `Redirect blocked: unsafe protocol in href "${redirect.options.href}". Allowed protocols: ${Array.from(this.protocolAllowlist).join(', ')}.`, | ||||||||||||||||
| ) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if (!redirect.headers.get('Location')) { | ||||||||||||||||
| redirect.headers.set('Location', redirect.options.href) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test case for
<Link />?Something like this