modelcontextprotocol / modelcontextprotocol/python-sdk

DCR registration accepts redirect_uris with non-HTTPS / non-loopback / fragmented schemes

Ouverte
#2,629 1 commentaire 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

auth bug fix proposed P2 ready for work
Langage dominant
Python
Étoiles
24.3k
Forks
4k
Merge moyen
1 j 1 h
PR mergées (30 j)
31

Description

Summary

The DCR handler (mcp.server.auth.handlers.register.RegistrationHandler.handle) does not validate the scheme of submitted redirect_uris. A client registered via DCR can supply javascript:, data:, vbscript:, file:, ftp:, or cleartext http:// (non-loopback) values, and they pass through to the provider's register_client. The SDK already enforces an HTTPS-or-loopback policy on the Issuer URL (routes.validate_issuer_url); the same policy is missing for registered redirect_uris. RFC 9700 §4.1.1 and RFC 7591 §2 require it.

Reproduction

The underlying field, mcp.shared.auth.OAuthClientMetadata.redirect_uris (src/mcp/shared/auth.py:40), is typed list[AnyUrl] | None. Pydantic's AnyUrl accepts any well-formed URL with a scheme. Verified on main at 161834d4ae:

from pydantic import AnyUrl, BaseModel, Field
from typing import List

class M(BaseModel):
    redirect_uris: List[AnyUrl] = Field(..., min_length=1)

for uri in [
    "javascript:alert(1)",
    "data:text/html,<script>alert(1)</script>",
    "file:///etc/passwd",
    "vbscript:msgbox(1)",
    "ftp://attacker.example/cb",
    "http://attacker.example/cb",
    "https://example.com/cb#frag",
    "https://example.com/cb#",
]:
    M(redirect_uris=[uri])  # all accepted, no ValidationError

Against a running MCP server with the default DCR handler, POST /register with any of the above values returns 201 and stores the URI. After registration, OAuthClientMetadata.validate_redirect_uri does exact-equality match against the registered list, so the bad URI is accepted as the authorization callback target.

Existing parallel logic to mirror

src/mcp/server/auth/routes.py:24–42 (validate_issuer_url):

if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
    raise ValueError("Issuer URL must be HTTPS")

if url.fragment:
    raise ValueError("Issuer URL must not have a fragment")

Related

  • #1446 (closed as duplicate, no cross-reference recorded) — raised the same concern in question form.
  • #1934 (open) — fixes RFC 8252 §7.3 loopback port matching at authorize-time. Adjacent surface but orthogonal; does not add scheme validation at register-time.
  • TypeScript SDK PR modelcontextprotocol/typescript-sdk#1738 covers the same authorize-time loopback gap on the TS side.

Proposed fix

Add validate_registered_redirect_uri(url: AnyUrl) -> None next to validate_issuer_url:

  • Reject schemes other than https, or http with host in {"localhost", "127.0.0.1", "[::1]"}.
  • Reject URIs with a fragment (including empty fragments, e.g. https://example.com/cb# — note: this is also a latent bug in validate_issuer_url's current if url.fragment: check, which I have NOT touched here to keep scope tight).
  • Permit query strings (RFC 7591 §2 explicitly allows them).

Call it once per URI in RegistrationHandler.handle immediately after model_validate_json succeeds. On failure return 400 invalid_redirect_uri per RFC 7591 §3.2.2.

PR with the patch + tests: #<PR_NUM_HERE>.

Notes on severity

Browsers no longer navigate javascript: / data: schemes received in Location headers, which neutralises those vectors for browser-mediated flows. The realistic exploitable residue is (a) cleartext-HTTP redirect_uris to attacker-controlled hosts, and (b) custom-scheme deep links on devices where the MCP client uses a system handler. Defense-in-depth, not a critical exploit chain — happy to be downgraded if maintainers see it differently.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Lisez src/mcp/server/auth/routes.py pour validate_issuer_url, puis examinez RegistrationHandler.handle et OAuthClientMetadata dans src/mcp/shared/auth.py. Reproduisez les cas d’URI de redirection fournis avec POST /register, ajoutez une couverture ciblée pour HTTPS accepté ou HTTP loopback ainsi que pour les schémas ou fragments rejetés, et confirmez que les entrées invalides renvoient 400 invalid_redirect_uri.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
authentication, security
Type d'issue
Bug
Difficulté
3/5
Temps estimé
1-2 jours
Activité
Calme
Clarté
Clairement spécifiée
Accessibilité débutants
67/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.