Skip to content

Commit ebbe83d

Browse files
cursoragentmsukkari
andcommitted
fix(web): prevent XSS via OAuth redirect URL validation
- Add validateOAuthRedirectUrl() utility to block dangerous protocols (javascript:, data:, vbscript:) while allowing safe ones (http:, https:, custom app protocols like cursor://, vscode://, claude://) - Validate redirect URLs in consentScreen.tsx before window.location.href navigation for both approve and deny flows - Remove error message interpolation in toast descriptions to prevent XSS via exception text (now uses static error messages) - Validate redirect URL in oauth/complete/page.tsx before navigation - Add comprehensive unit tests for the new validation function Fixes CodeQL alert #33: js/xss-through-exception Co-authored-by: Michael Sukkarieh <msukkari@users.noreply.github.com>
1 parent 2c89825 commit ebbe83d

4 files changed

Lines changed: 136 additions & 10 deletions

File tree

packages/web/src/app/oauth/authorize/components/consentScreen.tsx

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { approveAuthorization, denyAuthorization } from '@/ee/features/oauth/actions';
44
import { LoadingButton } from '@/components/ui/loading-button';
5-
import { isServiceError } from '@/lib/utils';
5+
import { isServiceError, validateOAuthRedirectUrl } from '@/lib/utils';
66
import { ClientIcon } from './clientIcon';
77
import Image from 'next/image';
88
import logo from '@/public/logo_512.png';
@@ -44,16 +44,24 @@ export function ConsentScreen({
4444
setPending('approve');
4545
const result = await approveAuthorization({ clientId, redirectUri, codeChallenge, resource, state });
4646
if (!isServiceError(result)) {
47-
toast({
48-
description: `✅ Authorization approved successfully. Redirecting...`,
49-
});
50-
window.location.href = result;
47+
const validatedUrl = validateOAuthRedirectUrl(result);
48+
if (validatedUrl) {
49+
toast({
50+
description: `✅ Authorization approved successfully. Redirecting...`,
51+
});
52+
window.location.href = validatedUrl;
53+
} else {
54+
toast({
55+
description: '❌ Invalid redirect URL. Authorization could not be completed.',
56+
});
57+
setPending(null);
58+
}
5159
} else {
5260
toast({
53-
description: `❌ Failed to approve authorization. ${result.message}`,
61+
description: '❌ Failed to approve authorization. Please try again.',
5462
});
63+
setPending(null);
5564
}
56-
setPending(null);
5765
};
5866

5967
const onDeny = async () => {
@@ -64,7 +72,15 @@ export function ConsentScreen({
6472
setPending(null);
6573
return;
6674
}
67-
window.location.href = result;
75+
const validatedUrl = validateOAuthRedirectUrl(result);
76+
if (validatedUrl) {
77+
window.location.href = validatedUrl;
78+
} else {
79+
toast({
80+
description: '❌ Invalid redirect URL. Could not complete the request.',
81+
});
82+
setPending(null);
83+
}
6884
};
6985

7086
return (

packages/web/src/app/oauth/complete/page.tsx

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,35 @@
11
'use client';
22

3-
import { useEffect } from 'react';
3+
import { useEffect, useState } from 'react';
4+
import { validateOAuthRedirectUrl } from '@/lib/utils';
45

56
// Handles the final redirect after OAuth authorization. For http/https redirect URIs,
67
// a server-side redirect is sufficient. For custom protocol URIs (e.g. cursor://, claude://),
78
// browsers won't follow HTTP redirects, so we use window.location.href instead.
89
export default function OAuthCompletePage() {
10+
const [error, setError] = useState<string | null>(null);
11+
912
useEffect(() => {
1013
const url = new URLSearchParams(window.location.search).get('url');
1114
if (url) {
12-
window.location.href = decodeURIComponent(url);
15+
const decodedUrl = decodeURIComponent(url);
16+
const validatedUrl = validateOAuthRedirectUrl(decodedUrl);
17+
if (validatedUrl) {
18+
window.location.href = validatedUrl;
19+
} else {
20+
setError('Invalid redirect URL. The authorization could not be completed.');
21+
}
1322
}
1423
}, []);
1524

25+
if (error) {
26+
return (
27+
<div className="min-h-screen flex items-center justify-center bg-background">
28+
<p className="text-sm text-destructive">{error}</p>
29+
</div>
30+
);
31+
}
32+
1633
return (
1734
<div className="min-h-screen flex items-center justify-center bg-background">
1835
<p className="text-sm text-muted-foreground">Redirecting, you may close this window...</p>

packages/web/src/lib/utils.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { expect, test, describe } from 'vitest';
2+
import { validateOAuthRedirectUrl } from './utils';
3+
4+
describe('validateOAuthRedirectUrl', () => {
5+
describe('blocks dangerous protocols', () => {
6+
test('rejects javascript: protocol', () => {
7+
expect(validateOAuthRedirectUrl('javascript:alert(1)')).toBeNull();
8+
expect(validateOAuthRedirectUrl('JavaScript:alert(1)')).toBeNull();
9+
expect(validateOAuthRedirectUrl('JAVASCRIPT:alert(document.cookie)')).toBeNull();
10+
});
11+
12+
test('rejects data: protocol', () => {
13+
expect(validateOAuthRedirectUrl('data:text/html,<script>alert(1)</script>')).toBeNull();
14+
expect(validateOAuthRedirectUrl('DATA:text/html,test')).toBeNull();
15+
});
16+
17+
test('rejects vbscript: protocol', () => {
18+
expect(validateOAuthRedirectUrl('vbscript:msgbox(1)')).toBeNull();
19+
expect(validateOAuthRedirectUrl('VBScript:test')).toBeNull();
20+
});
21+
});
22+
23+
describe('allows safe protocols', () => {
24+
test('allows https: protocol', () => {
25+
const result = validateOAuthRedirectUrl('https://example.com/callback?code=abc');
26+
expect(result).toBe('https://example.com/callback?code=abc');
27+
});
28+
29+
test('allows http: protocol', () => {
30+
const result = validateOAuthRedirectUrl('http://localhost:3000/callback');
31+
expect(result).toBe('http://localhost:3000/callback');
32+
});
33+
34+
test('allows custom app protocols (cursor://)', () => {
35+
const result = validateOAuthRedirectUrl('cursor://callback?code=abc');
36+
expect(result).toBe('cursor://callback?code=abc');
37+
});
38+
39+
test('allows custom app protocols (vscode://)', () => {
40+
const result = validateOAuthRedirectUrl('vscode://callback?code=abc');
41+
expect(result).toBe('vscode://callback?code=abc');
42+
});
43+
44+
test('allows custom app protocols (claude://)', () => {
45+
const result = validateOAuthRedirectUrl('claude://callback');
46+
expect(result).toBe('claude://callback');
47+
});
48+
});
49+
50+
describe('handles invalid URLs', () => {
51+
test('rejects malformed URLs', () => {
52+
expect(validateOAuthRedirectUrl('not a valid url')).toBeNull();
53+
expect(validateOAuthRedirectUrl('')).toBeNull();
54+
});
55+
});
56+
57+
describe('normalizes URLs', () => {
58+
test('returns normalized URL string', () => {
59+
const result = validateOAuthRedirectUrl('https://example.com:443/path');
60+
expect(result).toBe('https://example.com/path');
61+
});
62+
});
63+
});

packages/web/src/lib/utils.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,4 +589,34 @@ export const isHttpError = (error: unknown, status: number): boolean => {
589589
&& typeof error === 'object'
590590
&& 'status' in error
591591
&& error.status === status;
592+
}
593+
594+
/**
595+
* Validates an OAuth redirect URL to prevent open redirect and javascript: URI attacks.
596+
* Returns the validated URL if safe, or null if the URL is potentially malicious.
597+
*
598+
* Allowed protocols:
599+
* - https: (standard web URLs)
600+
* - http: (for localhost/development only)
601+
* - Custom protocols registered by OAuth clients (e.g., vscode://, cursor://, claude://)
602+
*
603+
* Blocked protocols:
604+
* - javascript: (XSS attack vector)
605+
* - data: (can execute scripts)
606+
* - vbscript: (legacy attack vector)
607+
*/
608+
export const validateOAuthRedirectUrl = (url: string): string | null => {
609+
try {
610+
const parsed = new URL(url);
611+
const protocol = parsed.protocol.toLowerCase();
612+
613+
const dangerousProtocols = ['javascript:', 'data:', 'vbscript:'];
614+
if (dangerousProtocols.includes(protocol)) {
615+
return null;
616+
}
617+
618+
return parsed.toString();
619+
} catch {
620+
return null;
621+
}
592622
}

0 commit comments

Comments
 (0)