modelcontextprotocol / modelcontextprotocol/python-sdk

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

Offen
#2,629 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

auth bug fix proposed P2 ready for work
Vorherrschende Sprache
Python
Sterne
24.3k
Forks
4k
Ø Merge
1 T. 1 Std.
Gemergte PRs (30 T.)
31

Beschreibung

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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Lesen Sie src/mcp/server/auth/routes.py für validate_issuer_url, und untersuchen Sie anschließend RegistrationHandler.handle und OAuthClientMetadata in src/mcp/shared/auth.py. Reproduzieren Sie die vorgegebenen Redirect-URI-Fälle gegen POST /register, fügen Sie eine gezielte Testabdeckung für akzeptiertes HTTPS oder Loopback-HTTP sowie abgelehnte Schemes oder Fragmente hinzu, und bestätigen Sie, dass ungültige Eingaben 400 invalid_redirect_uri zurückgeben.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python
Bereich
authentication, security
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
67/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.