modelcontextprotocol / modelcontextprotocol/python-sdk

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

Aperta
#2,629 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

auth bug fix proposed P2 ready for work
Lingua principale
Python
Stelle
24.3k
Fork
4k
Merge medio
1g 1h
PR unite (30g)
31

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Leggi src/mcp/server/auth/routes.py per validate_issuer_url, quindi esamina RegistrationHandler.handle e OAuthClientMetadata in src/mcp/shared/auth.py. Riproduci i casi di URI di reindirizzamento forniti con POST /register, aggiungi una copertura mirata per HTTPS accettato o HTTP loopback e per schemi o frammenti rifiutati, e conferma che gli input non validi restituiscano 400 invalid_redirect_uri.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
python
Ambito
authentication, security
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
67/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.