Skip to content

Commit 3305ecc

Browse files
cursoragentcursoragent
committed
Enhance URL parameter ferrying with robust security measures
1 parent 0501138 commit 3305ecc

File tree

3 files changed

+132
-19
lines changed

3 files changed

+132
-19
lines changed

‎URL_PARAMETER_FERRYING.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ import {NavLink} from 'sentry-docs/components/navlink';
9999
<NavLink href="/docs/guides/">Guides</NavLink>
100100
```
101101

102+
## Security Features
103+
104+
The implementation includes comprehensive security measures:
105+
- **URL Scheme Validation**: Blocks dangerous URL schemes (`javascript:`, `data:`, `vbscript:`, `file:`, `about:`)
106+
- **Parameter Sanitization**: Sanitizes parameter keys and values to prevent injection attacks
107+
- **Length Limits**: Parameter values are limited to 500 characters
108+
- **Control Character Filtering**: Removes control characters from parameters
109+
- **Multiple Validation Layers**: URLs are validated at multiple stages of processing
110+
102111
## Browser Compatibility
103112

104113
The implementation uses:

‎src/components/paramFerry.tsx‎

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,45 @@ export default function ParamFerry(): null {
2828
return key === pattern;
2929
});
3030

31-
if (shouldSync) {
32-
params[key] = value;
31+
if (shouldSync && typeof value === 'string' && typeof key === 'string') {
32+
// Sanitize parameter key and value during collection
33+
const sanitizedKey = key.replace(/[^\w\-._~]/g, '');
34+
const sanitizedValue = value.replace(/[\r\n\t]/g, '').substring(0, 500);
35+
36+
if (sanitizedKey && sanitizedValue) {
37+
params[sanitizedKey] = sanitizedValue;
38+
}
3339
}
3440
});
3541

3642
return params;
3743
};
3844

45+
// Validate URL is safe to process
46+
const isSafeUrl = (url: string): boolean => {
47+
// Block dangerous schemes
48+
const dangerousSchemes = ['javascript:', 'data:', 'vbscript:', 'file:', 'about:'];
49+
const lowerUrl = url.toLowerCase().trim();
50+
51+
if (dangerousSchemes.some(scheme => lowerUrl.startsWith(scheme))) {
52+
return false;
53+
}
54+
55+
// Only allow http, https, and relative URLs
56+
if (lowerUrl.startsWith('http://') || lowerUrl.startsWith('https://') || lowerUrl.startsWith('/') || !lowerUrl.includes(':')) {
57+
return true;
58+
}
59+
60+
return false;
61+
};
62+
3963
// Ferry parameters to a URL
4064
const ferryParams = (targetUrl: string) => {
65+
// Validate URL safety first
66+
if (!isSafeUrl(targetUrl)) {
67+
return targetUrl;
68+
}
69+
4170
const params = getCurrentParams();
4271

4372
if (Object.keys(params).length === 0) {
@@ -47,14 +76,32 @@ export default function ParamFerry(): null {
4776
try {
4877
const url = new URL(targetUrl, window.location.origin);
4978

50-
// Add parameters to the URL
79+
// Double-check the constructed URL is safe
80+
if (!isSafeUrl(url.toString())) {
81+
return targetUrl;
82+
}
83+
84+
// Add parameters to the URL with validation
5185
Object.entries(params).forEach(([key, value]) => {
52-
if (value) {
53-
url.searchParams.set(key, value);
86+
if (value && typeof value === 'string') {
87+
// Sanitize parameter key and value
88+
const sanitizedKey = key.replace(/[^\w\-._~]/g, '');
89+
const sanitizedValue = value.replace(/[\r\n\t]/g, '').substring(0, 500); // Limit length and remove control characters
90+
91+
if (sanitizedKey && sanitizedValue) {
92+
url.searchParams.set(sanitizedKey, sanitizedValue);
93+
}
5494
}
5595
});
5696

57-
return url.toString();
97+
const result = url.toString();
98+
99+
// Final safety check
100+
if (!isSafeUrl(result)) {
101+
return targetUrl;
102+
}
103+
104+
return result;
58105
} catch (error) {
59106
console.warn('Error ferrying parameters:', error);
60107
return targetUrl;
@@ -73,12 +120,13 @@ export default function ParamFerry(): null {
73120
// Skip if already processed
74121
if (anchor.hasAttribute('data-param-ferried')) return;
75122

76-
// Skip external links, anchors, mailto, and tel links
123+
// Skip external links, anchors, mailto, tel links, and unsafe URLs
77124
if (
78125
(href.startsWith('http') && !href.includes(window.location.hostname)) ||
79126
href.startsWith('#') ||
80127
href.startsWith('mailto:') ||
81-
href.startsWith('tel:')
128+
href.startsWith('tel:') ||
129+
!isSafeUrl(href)
82130
) {
83131
anchor.setAttribute('data-param-ferried', 'skip');
84132
return;
@@ -87,14 +135,20 @@ export default function ParamFerry(): null {
87135
try {
88136
const ferriedUrl = ferryParams(href);
89137

90-
if (ferriedUrl !== href) {
138+
if (ferriedUrl !== href && isSafeUrl(ferriedUrl)) {
91139
// For relative URLs, extract just the path and search params
92140
if (!href.startsWith('http')) {
93141
const url = new URL(ferriedUrl);
94142
const newHref = url.pathname + url.search + url.hash;
95-
anchor.setAttribute('href', newHref);
143+
// Final validation before setting href
144+
if (isSafeUrl(newHref)) {
145+
anchor.setAttribute('href', newHref);
146+
}
96147
} else {
97-
anchor.setAttribute('href', ferriedUrl);
148+
// Final validation before setting href
149+
if (isSafeUrl(ferriedUrl)) {
150+
anchor.setAttribute('href', ferriedUrl);
151+
}
98152
}
99153
}
100154

‎src/utils.ts‎

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,28 @@ export const marketingUrlParams = (): URLQueryObject => {
9191
return marketingParams;
9292
};
9393

94+
/**
95+
* Validate URL is safe to process (blocks dangerous schemes)
96+
* @param url - The URL to validate
97+
* @returns true if URL is safe, false otherwise
98+
*/
99+
const isSafeUrl = (url: string): boolean => {
100+
// Block dangerous schemes
101+
const dangerousSchemes = ['javascript:', 'data:', 'vbscript:', 'file:', 'about:'];
102+
const lowerUrl = url.toLowerCase().trim();
103+
104+
if (dangerousSchemes.some(scheme => lowerUrl.startsWith(scheme))) {
105+
return false;
106+
}
107+
108+
// Only allow http, https, and relative URLs
109+
if (lowerUrl.startsWith('http://') || lowerUrl.startsWith('https://') || lowerUrl.startsWith('/') || !lowerUrl.includes(':')) {
110+
return true;
111+
}
112+
113+
return false;
114+
};
115+
94116
/**
95117
* Ferry URL parameters from current page to target URL
96118
* @param targetUrl - The URL to append parameters to
@@ -102,23 +124,51 @@ export const ferryUrlParams = (targetUrl: string, additionalParams: URLQueryObje
102124
return targetUrl;
103125
}
104126

127+
// Validate URL safety first
128+
if (!isSafeUrl(targetUrl)) {
129+
return targetUrl;
130+
}
131+
105132
const currentParams = marketingUrlParams();
106133
const allParams = {...currentParams, ...additionalParams};
107134

108135
if (Object.keys(allParams).length === 0) {
109136
return targetUrl;
110137
}
111138

112-
const url = new URL(targetUrl, window.location.origin);
113-
114-
// Add parameters to the URL
115-
Object.entries(allParams).forEach(([key, value]) => {
116-
if (value && typeof value === 'string') {
117-
url.searchParams.set(key, value);
139+
try {
140+
const url = new URL(targetUrl, window.location.origin);
141+
142+
// Double-check the constructed URL is safe
143+
if (!isSafeUrl(url.toString())) {
144+
return targetUrl;
145+
}
146+
147+
// Add parameters to the URL with validation
148+
Object.entries(allParams).forEach(([key, value]) => {
149+
if (value && typeof value === 'string') {
150+
// Sanitize parameter key and value
151+
const sanitizedKey = key.replace(/[^\w\-._~]/g, '');
152+
const sanitizedValue = value.replace(/[\r\n\t]/g, '').substring(0, 500); // Limit length and remove control characters
153+
154+
if (sanitizedKey && sanitizedValue) {
155+
url.searchParams.set(sanitizedKey, sanitizedValue);
156+
}
157+
}
158+
});
159+
160+
const result = url.toString();
161+
162+
// Final safety check
163+
if (!isSafeUrl(result)) {
164+
return targetUrl;
118165
}
119-
});
120166

121-
return url.toString();
167+
return result;
168+
} catch (error) {
169+
console.warn('Error ferrying parameters:', error);
170+
return targetUrl;
171+
}
122172
};
123173

124174
export function captureException(exception: unknown): void {

0 commit comments

Comments
 (0)