modelcontextprotocol / modelcontextprotocol/python-sdk

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

Aberta
#2,629 1 comentário 0 reações 0 responsáveis Ver no GitHub

Ninguém assumiu esta issue ainda.

auth bug fix proposed P2 ready for work
Linguagem predominante
Python
Estrelas
24.3k
Forks
4k
Merge médio
1d 1h
PRs com merge (30d)
31

Descrição

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.

Guia de contribuição

Abrir o guia de contribuição

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Direção de pesquisa

Leia src/mcp/server/auth/routes.py para validate_issuer_url e, em seguida, inspecione RegistrationHandler.handle e OAuthClientMetadata em src/mcp/shared/auth.py. Reproduza os casos de URI de redirecionamento fornecidos contra POST /register, adicione cobertura focada para HTTPS aceito ou HTTP de loopback e para esquemas ou fragmentos rejeitados, e confirme que entradas inválidas retornam 400 invalid_redirect_uri.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
python
Domínio
authentication, security
Tipo de issue
Bug
Dificuldade
3/5
Tempo estimado
1-2 dias
Status de atividade
Pouca atividade
Clareza
Claramente especificada
Facilidade para iniciantes
67/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.