From 44bb310579f15259d935a998f5bdf188189b5a53 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 20 May 2025 16:42:22 -0700 Subject: [PATCH 1/9] Fix uncaught exception in MCP server --- src/mcp/shared/session.py | 56 +++++++---- tests/issues/test_malformed_input.py | 138 +++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 17 deletions(-) create mode 100644 tests/issues/test_malformed_input.py diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 90b4eb27c..b52cafc93 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -14,6 +14,7 @@ from mcp.shared.exceptions import McpError from mcp.shared.message import MessageMetadata, ServerMessageMetadata, SessionMessage from mcp.types import ( + INVALID_PARAMS, CancelledNotification, ClientNotification, ClientRequest, @@ -351,26 +352,47 @@ async def _receive_loop(self) -> None: if isinstance(message, Exception): await self._handle_incoming(message) elif isinstance(message.message.root, JSONRPCRequest): - validated_request = self._receive_request_type.model_validate( - message.message.root.model_dump( - by_alias=True, mode="json", exclude_none=True + try: + validated_request = self._receive_request_type.model_validate( + message.message.root.model_dump( + by_alias=True, mode="json", exclude_none=True + ) + ) + responder = RequestResponder( + request_id=message.message.root.id, + request_meta=validated_request.root.params.meta + if validated_request.root.params + else None, + request=validated_request, + session=self, + on_complete=lambda r: self._in_flight.pop( + r.request_id, None), ) - ) - responder = RequestResponder( - request_id=message.message.root.id, - request_meta=validated_request.root.params.meta - if validated_request.root.params - else None, - request=validated_request, - session=self, - on_complete=lambda r: self._in_flight.pop(r.request_id, None), - ) - self._in_flight[responder.request_id] = responder - await self._received_request(responder) + self._in_flight[responder.request_id] = responder + await self._received_request(responder) - if not responder._completed: # type: ignore[reportPrivateUsage] - await self._handle_incoming(responder) + if not responder._completed: # type: ignore[reportPrivateUsage] + await self._handle_incoming(responder) + except Exception as e: + # For request validation errors, send a proper JSON-RPC error + # response instead of crashing the server + logging.warning( + f"Failed to validate request: {e}. " + f"Message was: {message.message.root}" + ) + error_response = JSONRPCError( + jsonrpc="2.0", + id=message.message.root.id, + error=ErrorData( + code=INVALID_PARAMS, + message="Invalid request parameters", + data="", + ), + ) + session_message = SessionMessage( + message=JSONRPCMessage(error_response)) + await self._write_stream.send(session_message) elif isinstance(message.message.root, JSONRPCNotification): try: diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py new file mode 100644 index 000000000..5824cd9ee --- /dev/null +++ b/tests/issues/test_malformed_input.py @@ -0,0 +1,138 @@ +# Claude Debug +"""Test for HackerOne vulnerability report #3156202 - malformed input DOS.""" + +import anyio +import pytest + +from mcp.server.session import ServerSession +from mcp.shared.message import SessionMessage +from mcp.types import ( + INVALID_PARAMS, + JSONRPCError, + JSONRPCMessage, + JSONRPCRequest, +) + + +@pytest.mark.anyio +async def test_malformed_initialize_request_does_not_crash_server(): + """ + Test that malformed initialize requests return proper error responses + instead of crashing the server (HackerOne #3156202). + """ + # Create in-memory streams for testing + read_send_stream, read_receive_stream = anyio.create_memory_object_stream(10) + write_send_stream, write_receive_stream = anyio.create_memory_object_stream(10) + + # Create a malformed initialize request (missing required params field) + malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id="f20fe86132ed4cd197f89a7134de5685", + method="initialize", + # params=None # Missing required params field + ) + + # Wrap in session message + request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) + + # Start a server session + async with ServerSession( + read_stream=read_receive_stream, + write_stream=write_send_stream, + initialization_options={}, + ): + # Send the malformed request + await read_send_stream.send(request_message) + + # Give the session time to process the request + await anyio.sleep(0.1) + + # Check that we received an error response instead of a crash + try: + response_message = write_receive_stream.receive_nowait() + response = response_message.message.root + + # Verify it's a proper JSON-RPC error response + assert isinstance(response, JSONRPCError) + assert response.jsonrpc == "2.0" + assert response.id == "f20fe86132ed4cd197f89a7134de5685" + assert response.error.code == INVALID_PARAMS + assert "Invalid request parameters" in response.error.message + + # Verify the session is still alive and can handle more requests + # Send another malformed request to confirm server stability + another_malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id="test_id_2", + method="tools/call", + # params=None # Missing required params + ) + another_request_message = SessionMessage( + message=JSONRPCMessage(another_malformed_request) + ) + + await read_send_stream.send(another_request_message) + await anyio.sleep(0.1) + + # Should get another error response, not a crash + second_response_message = write_receive_stream.receive_nowait() + second_response = second_response_message.message.root + + assert isinstance(second_response, JSONRPCError) + assert second_response.id == "test_id_2" + assert second_response.error.code == INVALID_PARAMS + + except anyio.WouldBlock: + pytest.fail("No response received - server likely crashed") + + +@pytest.mark.anyio +async def test_multiple_concurrent_malformed_requests(): + """ + Test that multiple concurrent malformed requests don't crash the server. + """ + # Create in-memory streams for testing + read_send_stream, read_receive_stream = anyio.create_memory_object_stream(100) + write_send_stream, write_receive_stream = anyio.create_memory_object_stream(100) + + # Start a server session + async with ServerSession( + read_stream=read_receive_stream, + write_stream=write_send_stream, + initialization_options={}, + ): + # Send multiple malformed requests concurrently + malformed_requests = [] + for i in range(10): + malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id=f"malformed_{i}", + method="initialize", + # params=None # Missing required params + ) + request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) + malformed_requests.append(request_message) + + # Send all requests + for request in malformed_requests: + await read_send_stream.send(request) + + # Give time to process + await anyio.sleep(0.2) + + # Verify we get error responses for all requests + error_responses = [] + try: + while True: + response_message = write_receive_stream.receive_nowait() + error_responses.append(response_message.message.root) + except anyio.WouldBlock: + pass # No more messages + + # Should have received 10 error responses + assert len(error_responses) == 10 + + for i, response in enumerate(error_responses): + assert isinstance(response, JSONRPCError) + assert response.id == f"malformed_{i}" + assert response.error.code == INVALID_PARAMS \ No newline at end of file From 900548c602524984c0daf70d0c42d2b0c675103e Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 13:36:31 -0700 Subject: [PATCH 2/9] Fix tests --- tests/issues/test_malformed_input.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py index 5824cd9ee..51bbbcdd4 100644 --- a/tests/issues/test_malformed_input.py +++ b/tests/issues/test_malformed_input.py @@ -4,6 +4,7 @@ import anyio import pytest +from mcp.server.models import InitializationOptions from mcp.server.session import ServerSession from mcp.shared.message import SessionMessage from mcp.types import ( @@ -39,7 +40,7 @@ async def test_malformed_initialize_request_does_not_crash_server(): async with ServerSession( read_stream=read_receive_stream, write_stream=write_send_stream, - initialization_options={}, + init_options=InitializationOptions(), ): # Send the malformed request await read_send_stream.send(request_message) @@ -99,7 +100,7 @@ async def test_multiple_concurrent_malformed_requests(): async with ServerSession( read_stream=read_receive_stream, write_stream=write_send_stream, - initialization_options={}, + init_options=InitializationOptions(), ): # Send multiple malformed requests concurrently malformed_requests = [] From 69334862cacd055f6dcd74fdbe858dabae82cd74 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 13:38:36 -0700 Subject: [PATCH 3/9] Fix tests --- tests/issues/test_malformed_input.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py index 51bbbcdd4..0f90bd7fb 100644 --- a/tests/issues/test_malformed_input.py +++ b/tests/issues/test_malformed_input.py @@ -12,6 +12,7 @@ JSONRPCError, JSONRPCMessage, JSONRPCRequest, + ServerCapabilities, ) @@ -40,7 +41,11 @@ async def test_malformed_initialize_request_does_not_crash_server(): async with ServerSession( read_stream=read_receive_stream, write_stream=write_send_stream, - init_options=InitializationOptions(), + init_options=InitializationOptions( + server_name="test_server", + server_version="1.0.0", + capabilities=ServerCapabilities(), + ), ): # Send the malformed request await read_send_stream.send(request_message) @@ -100,7 +105,11 @@ async def test_multiple_concurrent_malformed_requests(): async with ServerSession( read_stream=read_receive_stream, write_stream=write_send_stream, - init_options=InitializationOptions(), + init_options=InitializationOptions( + server_name="test_server", + server_version="1.0.0", + capabilities=ServerCapabilities(), + ), ): # Send multiple malformed requests concurrently malformed_requests = [] From 9f83b507da43525f0f6e95681ec086d2b390bb89 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 13:42:39 -0700 Subject: [PATCH 4/9] Fix tests --- tests/issues/test_malformed_input.py | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py index 0f90bd7fb..f518a32ce 100644 --- a/tests/issues/test_malformed_input.py +++ b/tests/issues/test_malformed_input.py @@ -23,8 +23,12 @@ async def test_malformed_initialize_request_does_not_crash_server(): instead of crashing the server (HackerOne #3156202). """ # Create in-memory streams for testing - read_send_stream, read_receive_stream = anyio.create_memory_object_stream(10) - write_send_stream, write_receive_stream = anyio.create_memory_object_stream(10) + read_send_stream, read_receive_stream = anyio.create_memory_object_stream[ + SessionMessage | Exception + ](10) + write_send_stream, write_receive_stream = anyio.create_memory_object_stream[ + SessionMessage + ](10) # Create a malformed initialize request (missing required params field) malformed_request = JSONRPCRequest( @@ -90,6 +94,10 @@ async def test_malformed_initialize_request_does_not_crash_server(): except anyio.WouldBlock: pytest.fail("No response received - server likely crashed") + + # Close the send streams to signal end of communication + await read_send_stream.aclose() + await write_send_stream.aclose() @pytest.mark.anyio @@ -98,8 +106,12 @@ async def test_multiple_concurrent_malformed_requests(): Test that multiple concurrent malformed requests don't crash the server. """ # Create in-memory streams for testing - read_send_stream, read_receive_stream = anyio.create_memory_object_stream(100) - write_send_stream, write_receive_stream = anyio.create_memory_object_stream(100) + read_send_stream, read_receive_stream = anyio.create_memory_object_stream[ + SessionMessage | Exception + ](100) + write_send_stream, write_receive_stream = anyio.create_memory_object_stream[ + SessionMessage + ](100) # Start a server session async with ServerSession( @@ -145,4 +157,8 @@ async def test_multiple_concurrent_malformed_requests(): for i, response in enumerate(error_responses): assert isinstance(response, JSONRPCError) assert response.id == f"malformed_{i}" - assert response.error.code == INVALID_PARAMS \ No newline at end of file + assert response.error.code == INVALID_PARAMS + + # Close the send streams to signal end of communication + await read_send_stream.aclose() + await write_send_stream.aclose() \ No newline at end of file From 92865e891417dc9935cd28fc7004d935b375e90f Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 13:45:11 -0700 Subject: [PATCH 5/9] Fix tests --- tests/issues/test_malformed_input.py | 232 ++++++++++++++------------- 1 file changed, 119 insertions(+), 113 deletions(-) diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py index f518a32ce..ff8157daf 100644 --- a/tests/issues/test_malformed_input.py +++ b/tests/issues/test_malformed_input.py @@ -30,74 +30,77 @@ async def test_malformed_initialize_request_does_not_crash_server(): SessionMessage ](10) - # Create a malformed initialize request (missing required params field) - malformed_request = JSONRPCRequest( - jsonrpc="2.0", - id="f20fe86132ed4cd197f89a7134de5685", - method="initialize", - # params=None # Missing required params field - ) - - # Wrap in session message - request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) - - # Start a server session - async with ServerSession( - read_stream=read_receive_stream, - write_stream=write_send_stream, - init_options=InitializationOptions( - server_name="test_server", - server_version="1.0.0", - capabilities=ServerCapabilities(), - ), - ): - # Send the malformed request - await read_send_stream.send(request_message) - - # Give the session time to process the request - await anyio.sleep(0.1) + try: + # Create a malformed initialize request (missing required params field) + malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id="f20fe86132ed4cd197f89a7134de5685", + method="initialize", + # params=None # Missing required params field + ) - # Check that we received an error response instead of a crash - try: - response_message = write_receive_stream.receive_nowait() - response = response_message.message.root - - # Verify it's a proper JSON-RPC error response - assert isinstance(response, JSONRPCError) - assert response.jsonrpc == "2.0" - assert response.id == "f20fe86132ed4cd197f89a7134de5685" - assert response.error.code == INVALID_PARAMS - assert "Invalid request parameters" in response.error.message - - # Verify the session is still alive and can handle more requests - # Send another malformed request to confirm server stability - another_malformed_request = JSONRPCRequest( - jsonrpc="2.0", - id="test_id_2", - method="tools/call", - # params=None # Missing required params - ) - another_request_message = SessionMessage( - message=JSONRPCMessage(another_malformed_request) - ) + # Wrap in session message + request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) + + # Start a server session + async with ServerSession( + read_stream=read_receive_stream, + write_stream=write_send_stream, + init_options=InitializationOptions( + server_name="test_server", + server_version="1.0.0", + capabilities=ServerCapabilities(), + ), + ): + # Send the malformed request + await read_send_stream.send(request_message) - await read_send_stream.send(another_request_message) + # Give the session time to process the request await anyio.sleep(0.1) - # Should get another error response, not a crash - second_response_message = write_receive_stream.receive_nowait() - second_response = second_response_message.message.root - - assert isinstance(second_response, JSONRPCError) - assert second_response.id == "test_id_2" - assert second_response.error.code == INVALID_PARAMS - - except anyio.WouldBlock: - pytest.fail("No response received - server likely crashed") - - # Close the send streams to signal end of communication - await read_send_stream.aclose() - await write_send_stream.aclose() + # Check that we received an error response instead of a crash + try: + response_message = write_receive_stream.receive_nowait() + response = response_message.message.root + + # Verify it's a proper JSON-RPC error response + assert isinstance(response, JSONRPCError) + assert response.jsonrpc == "2.0" + assert response.id == "f20fe86132ed4cd197f89a7134de5685" + assert response.error.code == INVALID_PARAMS + assert "Invalid request parameters" in response.error.message + + # Verify the session is still alive and can handle more requests + # Send another malformed request to confirm server stability + another_malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id="test_id_2", + method="tools/call", + # params=None # Missing required params + ) + another_request_message = SessionMessage( + message=JSONRPCMessage(another_malformed_request) + ) + + await read_send_stream.send(another_request_message) + await anyio.sleep(0.1) + + # Should get another error response, not a crash + second_response_message = write_receive_stream.receive_nowait() + second_response = second_response_message.message.root + + assert isinstance(second_response, JSONRPCError) + assert second_response.id == "test_id_2" + assert second_response.error.code == INVALID_PARAMS + + except anyio.WouldBlock: + pytest.fail("No response received - server likely crashed") + finally: + # Close all streams to ensure proper cleanup + await read_send_stream.aclose() + await write_send_stream.aclose() + await read_receive_stream.aclose() + await write_receive_stream.aclose() @pytest.mark.anyio @@ -113,52 +116,55 @@ async def test_multiple_concurrent_malformed_requests(): SessionMessage ](100) - # Start a server session - async with ServerSession( - read_stream=read_receive_stream, - write_stream=write_send_stream, - init_options=InitializationOptions( - server_name="test_server", - server_version="1.0.0", - capabilities=ServerCapabilities(), - ), - ): - # Send multiple malformed requests concurrently - malformed_requests = [] - for i in range(10): - malformed_request = JSONRPCRequest( - jsonrpc="2.0", - id=f"malformed_{i}", - method="initialize", - # params=None # Missing required params - ) - request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) - malformed_requests.append(request_message) - - # Send all requests - for request in malformed_requests: - await read_send_stream.send(request) - - # Give time to process - await anyio.sleep(0.2) - - # Verify we get error responses for all requests - error_responses = [] - try: - while True: - response_message = write_receive_stream.receive_nowait() - error_responses.append(response_message.message.root) - except anyio.WouldBlock: - pass # No more messages - - # Should have received 10 error responses - assert len(error_responses) == 10 - - for i, response in enumerate(error_responses): - assert isinstance(response, JSONRPCError) - assert response.id == f"malformed_{i}" - assert response.error.code == INVALID_PARAMS - - # Close the send streams to signal end of communication - await read_send_stream.aclose() - await write_send_stream.aclose() \ No newline at end of file + try: + # Start a server session + async with ServerSession( + read_stream=read_receive_stream, + write_stream=write_send_stream, + init_options=InitializationOptions( + server_name="test_server", + server_version="1.0.0", + capabilities=ServerCapabilities(), + ), + ): + # Send multiple malformed requests concurrently + malformed_requests = [] + for i in range(10): + malformed_request = JSONRPCRequest( + jsonrpc="2.0", + id=f"malformed_{i}", + method="initialize", + # params=None # Missing required params + ) + request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) + malformed_requests.append(request_message) + + # Send all requests + for request in malformed_requests: + await read_send_stream.send(request) + + # Give time to process + await anyio.sleep(0.2) + + # Verify we get error responses for all requests + error_responses = [] + try: + while True: + response_message = write_receive_stream.receive_nowait() + error_responses.append(response_message.message.root) + except anyio.WouldBlock: + pass # No more messages + + # Should have received 10 error responses + assert len(error_responses) == 10 + + for i, response in enumerate(error_responses): + assert isinstance(response, JSONRPCError) + assert response.id == f"malformed_{i}" + assert response.error.code == INVALID_PARAMS + finally: + # Close all streams to ensure proper cleanup + await read_send_stream.aclose() + await write_send_stream.aclose() + await read_receive_stream.aclose() + await write_receive_stream.aclose() \ No newline at end of file From aba774d6737ee3e097c4af1fa873de0bb0abd2f6 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 13:47:05 -0700 Subject: [PATCH 6/9] Fix tests --- tests/issues/test_malformed_input.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/issues/test_malformed_input.py b/tests/issues/test_malformed_input.py index ff8157daf..e4fda9e13 100644 --- a/tests/issues/test_malformed_input.py +++ b/tests/issues/test_malformed_input.py @@ -136,7 +136,9 @@ async def test_multiple_concurrent_malformed_requests(): method="initialize", # params=None # Missing required params ) - request_message = SessionMessage(message=JSONRPCMessage(malformed_request)) + request_message = SessionMessage( + message=JSONRPCMessage(malformed_request) + ) malformed_requests.append(request_message) # Send all requests From 3ecc02a321b43e0a07fba07d4642543dae2f30e4 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 17:20:23 -0700 Subject: [PATCH 7/9] Move extra debug information to logging.debug --- src/mcp/shared/session.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index b52cafc93..d79c4472b 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -377,9 +377,9 @@ async def _receive_loop(self) -> None: except Exception as e: # For request validation errors, send a proper JSON-RPC error # response instead of crashing the server - logging.warning( - f"Failed to validate request: {e}. " - f"Message was: {message.message.root}" + logging.warning(f"Failed to validate request: {e}") + logging.debug( + f"Message that failed validation: {message.message.root}" ) error_response = JSONRPCError( jsonrpc="2.0", From 3757bad31c0a5e67d73f3d5617afbb35503218a1 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Tue, 27 May 2025 17:45:54 -0700 Subject: [PATCH 8/9] Fix lint --- src/mcp/shared/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/shared/session.py b/src/mcp/shared/session.py index 0cafd71a7..28692331f 100644 --- a/src/mcp/shared/session.py +++ b/src/mcp/shared/session.py @@ -14,8 +14,8 @@ from mcp.shared.exceptions import McpError from mcp.shared.message import MessageMetadata, ServerMessageMetadata, SessionMessage from mcp.types import ( - INVALID_PARAMS, CONNECTION_CLOSED, + INVALID_PARAMS, CancelledNotification, ClientNotification, ClientRequest, From 605a6d4435dc56dfc5a35e8d1384f18ea66c7514 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Wed, 28 May 2025 10:11:24 -0700 Subject: [PATCH 9/9] Rerun tests