Skip to content

Commit d3ef532

Browse files
committed
fix(mcp): add CRLF injection protection and remove duplicate code
1 parent a675aaf commit d3ef532

4 files changed

Lines changed: 60 additions & 7 deletions

File tree

src/google/adk/runners.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -618,12 +618,6 @@ async def _run_with_trace(
618618
# Directly return if the current agent in invocation context is
619619
# already final.
620620
return
621-
if invocation_context.end_of_agents.get(
622-
invocation_context.agent.name
623-
):
624-
# Directly return if the current agent in invocation context is
625-
# already final.
626-
return
627621

628622
async def execute(ctx: InvocationContext) -> AsyncGenerator[Event]:
629623
async with Aclosing(ctx.agent.run_async(ctx)) as agen:

src/google/adk/tools/mcp_tool/_internal.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,26 @@ def validate_header_name(header_name: str) -> None:
138138
)
139139

140140

141+
def validate_header_format(header_format: str) -> None:
142+
"""Validates that a header format string doesn't contain CRLF injection.
143+
144+
This prevents header injection attacks where malicious format strings
145+
could inject additional headers via CRLF sequences.
146+
147+
Args:
148+
header_format: The format string to validate.
149+
150+
Raises:
151+
ValueError: If header_format contains CRLF sequences.
152+
"""
153+
if "\r" in header_format or "\n" in header_format:
154+
raise ValueError(
155+
"Header format string cannot contain CRLF (carriage return or line feed) "
156+
"characters due to header injection risk. "
157+
f"Invalid format: {repr(header_format)}"
158+
)
159+
160+
141161
def _validate_header_value(value: Any, allow_binary: bool = False) -> None:
142162
"""Validates header values with RFC 7230 compliance and proper binary handling.
143163

src/google/adk/tools/mcp_tool/mcp_toolset.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
from ..tool_configs import BaseToolConfig
5050
from ..tool_configs import ToolArgsConfig
5151
from ._internal import sanitize_header_value
52+
from ._internal import validate_header_format
5253
from ._internal import validate_header_name
5354
from ._internal import validate_header_value
5455
from .mcp_session_manager import MCPSessionManager
@@ -144,8 +145,9 @@ def create_session_state_header_provider(
144145
)
145146
)
146147
"""
147-
# Validate header name upfront
148+
# Validate header name and format upfront to prevent injection attacks
148149
validate_header_name(header_name)
150+
validate_header_format(header_format)
149151

150152
def provider(ctx: ReadonlyContext) -> Dict[str, str]:
151153
value = ctx.state.get(state_key, default_value)

tests/unittests/tools/mcp_tool/test_jwt_token_propagation.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,3 +656,40 @@ def test_binary_data_handling(self):
656656
dangerous_binary = b"dangerous\x00data\x01with\x02control"
657657
with pytest.raises(ValueError, match="dangerous"):
658658
_validate_header_value(dangerous_binary, allow_binary=True)
659+
660+
def test_header_format_crlf_injection_protection(self):
661+
"""Test that header format strings with CRLF sequences are rejected."""
662+
from google.adk.tools.mcp_tool.mcp_toolset import create_session_state_header_provider
663+
from google.adk.tools.mcp_tool._internal import validate_header_format
664+
665+
# Valid format strings should be accepted
666+
valid_formats = [
667+
"Bearer {value}",
668+
"Basic {value}",
669+
"key:{value}",
670+
"Token {value}",
671+
"{value}",
672+
]
673+
for fmt in valid_formats:
674+
validate_header_format(fmt) # Should not raise
675+
676+
# Format strings with CRLF should be rejected
677+
invalid_formats = [
678+
"Bearer {value}\r\nX-Injected: evil",
679+
"Bearer {value}\nInjected-Header: bad",
680+
"Bearer {value}\rAnother: header",
681+
"Bearer {value}\r\n",
682+
"Bearer {value}\n",
683+
"Bearer {value}\r",
684+
]
685+
for fmt in invalid_formats:
686+
with pytest.raises(ValueError, match="CRLF"):
687+
validate_header_format(fmt)
688+
689+
# Test that create_session_state_header_provider validates format
690+
with pytest.raises(ValueError, match="CRLF"):
691+
create_session_state_header_provider(
692+
state_key="token",
693+
header_name="Authorization",
694+
header_format="Bearer {value}\r\nX-Injected: evil"
695+
)

0 commit comments

Comments
 (0)