Apply the request body limit to the SSE and OAuth endpoints - #3336
Apply the request body limit to the SSE and OAuth endpoints#3336maxisbey wants to merge 3 commits into
Conversation
SseServerTransport now takes max_request_body_size (default 4 MiB, the same default and validation as StreamableHTTPSessionManager) and answers 413 before session lookup or parsing when a POST declares or streams a larger body. The message endpoint only ever handled POST bodies, so it now answers 405 (Allow: POST) to other methods instead of treating them like a POST. MCPServer.sse_app(), run_sse_async() and run(transport="sse") expose the keyword, mirroring streamable_http_app().
create_auth_routes now wraps its endpoints in RequestBodyLimitMiddleware, so /token, /revoke, /register and POST /authorize answer 413 to bodies over the 4 MiB default before any form or JSON parsing. The limit sits inside the CORS wrapper so browser clients still get CORS headers on the 413; GET and OPTIONS requests pass through untouched.
📚 Documentation preview
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding, I also looked at whether the new SSE RequestBodyLimitMiddleware buffering the request body before the DNS-rebinding/session-ownership checks creates a new resource risk — the buffer is capped at max_request_body_size (4 MiB default) per request, so memory is bounded; it does mean up to that much body is read before the security checks that previously rejected without reading any body bytes, which is a behavior change worth a human glance rather than a bug.
Extended reasoning...
Findings were reported, so this is the brief ruled-out note only. The SSE path in src/mcp/server/sse.py now routes POSTs through RequestBodyLimitMiddleware before _handle_post_message runs its transport-security and session checks; I read the middleware in src/mcp/server/streamable_http_manager.py and confirmed it rejects once the accumulated body exceeds max_body_size, so buffering is bounded and not a memory-exhaustion vector — only an ordering change (body read before header-based rejections). The hunt exited on max_rounds and touches auth/security paths, so approval is off the table regardless; the inline comment already signals human review is needed.
| def _body_limited(handler: Callable[[Request], Response | Awaitable[Response]]) -> ASGIApp: | ||
| """Wrap an endpoint so POST bodies over the default limit are answered with 413 before it runs.""" | ||
| return RequestBodyLimitMiddleware(request_response(handler), DEFAULT_MAX_REQUEST_BODY_SIZE) |
There was a problem hiding this comment.
🔴 The new OAuth request-body limit only guards POST, but the wrapped routes accept other methods whose handlers still read the full body — so the 413 protection is bypassed by switching the method. RequestBodyLimitMiddleware.__call__ passes any non-POST request straight through (src/mcp/server/streamable_http_manager.py:382 if scope["type"] != "http" or scope["method"] != "POST"), and _body_limited relies on it. Yet /token, /register, and /revoke are registered with methods=["POST", "OPTIONS"], and a plain OPTIONS request without an Origin header (or without Access-Control-Request-Method) is not a CORS preflight, so CORSMiddleware forwards it to the handler. All three handlers read the body unconditionally: RegistrationHandler.handle calls `await…
Extended reasoning...
An unauthenticated attacker sends OPTIONS /register with no Origin header, Content-Type: application/json, and a multi-gigabyte (e.g. chunked) body. CORSMiddleware passes it through (not a preflight), the Route allows OPTIONS, RequestBodyLimitMiddleware skips it because the method is not POST, and RegistrationHandler.handle executes await request.body(), buffering the entire attacker-controlled body in server memory. The same works on /token and /revoke with Content-Type: application/x-www-form-urlencoded (Starlette's form() reads the whole body into memory), and on /authorize via HEAD. The 4 MiB cap this PR advertises for the OAuth endpoints (test: "rejects one over 4 MiB before parsing it") is therefore trivially bypassed, allowing memory-exhaustion DoS against the authorization server.
Verification: normal — the bypass is real: the guard this PR adds is method-gated to POST while the wrapped routes accept other methods whose handlers read the body unconditionally. Chain of citations: 1. /home/claude/python-sdk/src/mcp/server/streamable_http_manager.py:382 — if scope["type"] != "http" or scope["method"] != "POST": await self.app(scope, receive, send); return — RequestBodyLimitMiddlew
1 participant
#3095 added
RequestBodyLimitMiddlewareand applied it to the Streamable HTTP endpoint. This does the same for the other two places that accept POST bodies, so every HTTP entry point shares the one 4 MiB default.Motivation and Context
SseServerTransporttakesmax_request_body_size(default 4 MiB, same validation asStreamableHTTPSessionManager), andMCPServer.sse_app()/run(transport="sse")pass it through, mirroringstreamable_http_app(). The message endpoint now answers 405 to anything that isn't a POST instead of treating it as one.create_auth_routesendpoints (/token,/revoke,/register, POST/authorize) use the default limit. It sits inside the CORS wrapper, so a 413 still carries CORS headers and preflights are untouched.Nothing changes for requests under the limit.
How Has This Been Tested?
New tests in
tests/server/test_sse_security.py,tests/server/auth/test_error_handling.pyandtests/server/mcpserver/test_server.py: over-limit bodies (declared and streamed) get 413, bodies under the limit still reach session lookup / form parsing,OPTIONSpreflights pass through, non-POST to the message endpoint gets 405, andsse_app()applies the configured value. Full suite, pyright and ruff pass locally.Breaking Changes
None. The new keyword is optional and defaults to the limit
streamable_http_app()already uses; the only observable difference is a 413 for POST bodies over 4 MiB on these endpoints and a 405 for non-POST requests to the SSE message endpoint.Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
docs/migration.mdanddocs/run/index.mdnote thattransport="sse"takes the same keyword.AI Disclaimer