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

Activity

1. maxisbey

   added

   triageQueued for automated analysis — bot will process and remove this labelQueued for automated analysis — bot will process and remove this label

   on May 31, 2026
2. mcp-claude commented on Jun 1, 2026

   

   mcp-claudebot

   on Jun 1, 2026 – with mcp-claude

   More actions

   confirmed on main at 616476f and on v1.x at 6213787. OAuthClientMetadata.redirect_uris is list[AnyUrl] | None (src/mcp/shared/auth.py:40), which pydantic accepts for any well-formed URL scheme, and RegistrationHandler.handle (src/mcp/server/auth/handlers/register.py:32-127) does not apply any scheme/host/fragment policy before passing the URIs to register_client. POST /register returns 201 for javascript:, data:, file:, vbscript:, ftp:, cleartext http:// (non-loopback), and fragmented https://. fix is small and non-breaking: a validate_registered_redirect_uri helper called per URI right after model_validate_json succeeds, returning 400 invalid_redirect_uri on failure. needs backport to v1.x.

   repro.py

   """Reproduces issue #2629: DCR handler accepts dangerous redirect_uri schemes."""
   
   import asyncio
   import json
   from typing import Any
   
   from starlette.requests import Request
   
   from mcp.server.auth.handlers.register import RegistrationHandler
   from mcp.server.auth.provider import OAuthAuthorizationServerProvider
   from mcp.server.auth.settings import ClientRegistrationOptions
   from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata
   
   DANGEROUS = [
       "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",
   ]
   
   
   def part1_pydantic_accepts() -> None:
       print("=== part 1: OAuthClientMetadata.model_validate accepts dangerous schemes ===")
       for uri in DANGEROUS:
           try:
               m = OAuthClientMetadata.model_validate({"redirect_uris": [uri]})
               print(f"  ACCEPTED: {uri!r} -> stored as {m.redirect_uris[0]!s}")
           except Exception as exc:
               print(f"  rejected: {uri!r} -> {exc}")
   
   
   class StubProvider(OAuthAuthorizationServerProvider[Any, Any, Any]):
       def __init__(self) -> None:
           self.registered: list[OAuthClientInformationFull] = []
   
       async def get_client(self, client_id: str) -> OAuthClientInformationFull | None:
           for c in self.registered:
               if c.client_id == client_id:
                   return c
           return None
   
       async def register_client(self, client_info: OAuthClientInformationFull) -> None:
           self.registered.append(client_info)
   
       async def authorize(self, *a, **k): raise NotImplementedError
       async def load_authorization_code(self, *a, **k): raise NotImplementedError
       async def exchange_authorization_code(self, *a, **k): raise NotImplementedError
       async def load_refresh_token(self, *a, **k): raise NotImplementedError
       async def exchange_refresh_token(self, *a, **k): raise NotImplementedError
       async def load_access_token(self, *a, **k): raise NotImplementedError
       async def revoke_token(self, *a, **k): raise NotImplementedError
   
   
   def make_request(body: bytes) -> Request:
       scope = {
           "type": "http",
           "method": "POST",
           "path": "/register",
           "headers": [(b"content-type", b"application/json")],
           "query_string": b"",
       }
       received = False
   
       async def receive():
           nonlocal received
           if not received:
               received = True
               return {"type": "http.request", "body": body, "more_body": False}
           return {"type": "http.disconnect"}
   
       return Request(scope, receive)
   
   
   async def part2_handler_returns_201() -> None:
       print("\n=== part 2: RegistrationHandler.handle returns 201 for dangerous redirect_uris ===")
       provider = StubProvider()
       handler = RegistrationHandler(provider=provider, options=ClientRegistrationOptions())
   
       for uri in DANGEROUS:
           body = json.dumps({
               "redirect_uris": [uri],
               "grant_types": ["authorization_code", "refresh_token"],
               "response_types": ["code"],
               "token_endpoint_auth_method": "client_secret_post",
           }).encode()
           resp = await handler.handle(make_request(body))
           body_text = resp.body
           if isinstance(body_text, (bytes, bytearray)):
               body_text = body_text.decode()
           parsed = json.loads(body_text)
           print(f"  {uri!r}: status={resp.status_code}")
           if resp.status_code == 201:
               print(f"    -> client_id={parsed['client_id']} redirect_uris={parsed['redirect_uris']}")
           else:
               print(f"    -> error={parsed}")
   
   
   if __name__ == "__main__":
       part1_pydantic_accepts()
       asyncio.run(part2_handler_returns_201())

   command + output

   $ uv run python repro.py
   === part 1: OAuthClientMetadata.model_validate accepts dangerous schemes ===
     ACCEPTED: 'javascript:alert(1)' -> stored as javascript:alert(1)
     ACCEPTED: 'data:text/html,<script>alert(1)</script>' -> stored as data:text/html,<script>alert(1)</script>
     ACCEPTED: 'file:///etc/passwd' -> stored as file:///etc/passwd
     ACCEPTED: 'vbscript:msgbox(1)' -> stored as vbscript:msgbox(1)
     ACCEPTED: 'ftp://attacker.example/cb' -> stored as ftp://attacker.example/cb
     ACCEPTED: 'http://attacker.example/cb' -> stored as http://attacker.example/cb
     ACCEPTED: 'https://example.com/cb#frag' -> stored as https://example.com/cb#frag
   
   === part 2: RegistrationHandler.handle returns 201 for dangerous redirect_uris ===
     'javascript:alert(1)': status=201
     'data:text/html,<script>alert(1)</script>': status=201
     'file:///etc/passwd': status=201
     'vbscript:msgbox(1)': status=201
     'ftp://attacker.example/cb': status=201
     'http://attacker.example/cb': status=201
     'https://example.com/cb#frag': status=201
   

   code path

   • src/mcp/shared/auth.py:40 — redirect_uris: list[AnyUrl] | None = Field(..., min_length=1). AnyUrl accepts any RFC 3986 URL regardless of scheme.
   • src/mcp/server/auth/handlers/register.py:36 — OAuthClientMetadata.model_validate_json(body) succeeds for any of the above.
   • src/mcp/server/auth/handlers/register.py:100-124 — values are passed straight to provider.register_client with no scheme/host/fragment check.
   • src/mcp/server/auth/routes.py:24-42 has the parallel validate_issuer_url for the issuer URL; the same policy is not applied to registered redirect_uris.
   • Later, OAuthClientMetadata.validate_redirect_uri (auth.py:98+) does exact-equality matching against the stored list, so anything stored at register-time is accepted as the authorize-time callback target.
   • tests/interaction/_requirements.py:2049 already records this gap as a divergence on hosting:auth:as:redirect-uri-scheme, and tests/interaction/auth/test_as_handlers.py:282 pins the buggy 201 behavior — both flip to the compliant shape with the fix.

   suggested fix

   // src/mcp/server/auth/handlers/register.py
   -from pydantic import BaseModel, ValidationError
   +from pydantic import AnyUrl, BaseModel, ValidationError
    from starlette.requests import Request
    from starlette.responses import Response
   
    from mcp.server.auth.errors import stringify_pydantic_error
    ...
   +_UNSAFE_REDIRECT_SCHEMES = frozenset({"javascript", "data", "vbscript", "file"})
   +
   +
   +def validate_registered_redirect_uri(url: AnyUrl) -> None:
   +    scheme = (url.scheme or "").lower()
   +    if scheme in _UNSAFE_REDIRECT_SCHEMES:
   +        raise ValueError(f"redirect_uri scheme '{scheme}' is not allowed")
   +    if scheme == "http" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
   +        raise ValueError("redirect_uri 'http' scheme is only allowed for loopback hosts")
   +    if url.fragment:
   +        raise ValueError("redirect_uri must not have a fragment")
   +
    ...
            except ValidationError as validation_error:
                return PydanticJSONResponse(...)
   +
   +        for redirect_uri in client_metadata.redirect_uris or []:
   +            try:
   +                validate_registered_redirect_uri(redirect_uri)
   +            except ValueError as exc:
   +                return PydanticJSONResponse(
   +                    content=RegistrationErrorResponse(
   +                        error="invalid_redirect_uri",
   +                        error_description=str(exc),
   +                    ),
   +                    status_code=400,
   +                )

   test to verify: a parametrized integration test posts each of javascript:, data:, file:, vbscript:, non-loopback http://, and fragmented https:// to /register and asserts a 400 with error == "invalid_redirect_uri"; unit tests cover the helper directly (https / loopback / custom-scheme accepted; unsafe-scheme, non-loopback http, and fragment rejected). custom schemes like com.example.app:/oauth/cb and http://[::1]:8080/cb stay accepted so RFC 8252 native-app flows keep working. 1757 passed / 98 skipped / 1 xfailed across the full suite, including the flipped hosting:auth:as:redirect-uri-scheme divergence pin in tests/interaction/auth/test_as_handlers.py.
3. mcp-claude

   added

   bugSomething isn't workingSomething isn't working

   ready for workEnough information for someone to start working onEnough information for someone to start working on

   authIssues and PRs related to Authentication / OAuthIssues and PRs related to Authentication / OAuth

   P2Moderate issues affecting some users, edge cases, potentially valuable featureModerate issues affecting some users, edge cases, potentially valuable feature

   fix proposedBot has a verified fix diff in the commentBot has a verified fix diff in the comment

   and removed

   triageQueued for automated analysis — bot will process and remove this labelQueued for automated analysis — bot will process and remove this label

   on Jun 1, 2026
4. maxisbey

   added 3 commits that reference this issue on Jun 26, 2026

   Reject non-HTTPS, non-loopback redirect URIs at client registration

   ...

   This issue will close once commit 5dfcaea is merged into the 'main' branch.

   5dfcaea

   Reject non-HTTPS, non-loopback redirect URIs at client registration

   ...

   This issue will close once commit 1f72f51 is merged into the 'main' branch.

   1f72f51

   Reject non-HTTPS, non-loopback redirect URIs at client registration

   ...

   This issue will close once commit 24061fe is merged into the 'main' branch.

   24061fe