modelcontextprotocol / modelcontextprotocol/python-sdk

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

Đang mở
#2,629 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

auth bug fix proposed P2 ready for work
Ngôn ngữ chính
Python
Star
24.3k
Fork
4k
Merge trung bình
1 ngày 1 giờ
Pull request đã merge (30 ngày)
31

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Đọc src/mcp/server/auth/routes.py để tìm validate_issuer_url, sau đó kiểm tra RegistrationHandler.handle và OAuthClientMetadata trong src/mcp/shared/auth.py. Tái hiện các trường hợp URI chuyển hướng được cung cấp với POST /register, bổ sung coverage tập trung cho HTTPS hoặc HTTP loopback được chấp nhận và các scheme hoặc fragment bị từ chối, đồng thời xác nhận rằng các đầu vào không hợp lệ trả về 400 invalid_redirect_uri.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
authentication, security
Loại issue
Lỗi
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
67/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.