Skip to content

Commit 715b407

Browse files
security: tool description pinning, injection scan, DNS rebinding default
Three security fixes identified via full static audit of the Python SDK: 1. client/session.py — add protocol-version-agnostic security pass to _absorb_tool_listing(): SHA-256 description pinning blocks rug-pull attacks (tools/list_changed re-fetch accepts poisoned descriptions silently); regex injection scan drops tools with prompt injection patterns on first sight. 2. server/transport_security.py — flip enable_dns_rebinding_protection default from False to True; add startup WARNING when allowed_hosts is empty so misconfigured deployments fail visibly, not silently. 3. cli/cli.py — remove shell=True from Windows npx probe subprocess call (CWE-78; cmd values are hardcoded today but shell=True with a list is exploitable if ever made configurable). Signed-off-by: Nicholas Michael Kloster <nicholas@nuclide-research.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2af9e30 commit 715b407

3 files changed

Lines changed: 65 additions & 3 deletions

File tree

‎src/mcp/cli/cli.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def _get_npx_command():
4545
# Try both npx.cmd and npx.exe on Windows
4646
for cmd in ["npx.cmd", "npx.exe", "npx"]:
4747
try:
48-
subprocess.run([cmd, "--version"], check=True, capture_output=True, shell=True)
48+
subprocess.run([cmd, "--version"], check=True, capture_output=True)
4949
return cmd
5050
except subprocess.CalledProcessError:
5151
continue

‎src/mcp/client/session.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
from __future__ import annotations
22

3+
import hashlib
34
import json
45
import logging
6+
import re
57
from collections.abc import Callable, Mapping, Sequence
68
from dataclasses import dataclass
79
from functools import cache, reduce
@@ -110,6 +112,32 @@ def _later_revision_fields(method: str, version: str) -> frozenset[str]:
110112
return frozenset(later) - _wire_fields(current)
111113

112114

115+
_INJECTION_PATTERNS: tuple[re.Pattern[str], ...] = (
116+
re.compile(r"(?i)(ignore|disregard)\s+(all\s+)?(previous|prior|above)\s+(instructions?|prompts?|rules?|context)"),
117+
re.compile(r"(?i)(you\s+are\s+now|act\s+as|pretend\s+(to\s+be|you\s+are))\s+\w"),
118+
re.compile(r"(?i)(system\s*:?\s*override|admin\s+mode|unrestricted\s+mode|jailbreak)"),
119+
re.compile(r"(?i)(before\s+(any|every|all)\s+(operation|request|task|call)|critical\s+security\s+policy)"),
120+
re.compile(r"(?i)(exfiltrate|send\s+to\s+attacker|include\s+(in\s+)?.*response.*key|leak)"),
121+
re.compile(r"(?i)(read\s+~/\.(ssh|aws|kube|gnupg|config)|/etc/passwd|/proc/self)"),
122+
re.compile(r"(?i)(do\s+not\s+(mention|tell|show|disclose)\s+(this|the user|to user))"),
123+
re.compile(r"(?i)(\[IMPORTANT\s*:\s*[^\]]{20,}\])"),
124+
)
125+
126+
127+
def _tool_desc_hash(description: str) -> str:
128+
"""Return a short SHA-256 fingerprint of a tool description for rug-pull pinning."""
129+
return hashlib.sha256(description.encode()).hexdigest()[:16]
130+
131+
132+
def _scan_tool_description(description: str) -> str | None:
133+
"""Return the matched text if the description contains a known injection pattern, else None."""
134+
for pat in _INJECTION_PATTERNS:
135+
m = pat.search(description)
136+
if m:
137+
return m.group(0)[:80]
138+
return None
139+
140+
113141
def _same_schema(a: dict[str, Any] | None, b: dict[str, Any] | None) -> bool:
114142
"""JSON equality for two output schemas.
115143
@@ -442,6 +470,9 @@ def __init__(
442470
# `_absorb_tool_listing`, which evicts a tool's entry whenever its schema changes.
443471
self._tool_output_validators: dict[str, Validator] = {}
444472
self._x_mcp_header_maps: dict[str, dict[tuple[str, ...], str]] = {}
473+
# Pinned description hashes for rug-pull detection (tool name → SHA-256[:16]).
474+
# Set on first listing; any subsequent change is a security event.
475+
self._pinned_tool_desc_hashes: dict[str, str] = {}
445476
self._initialize_result: types.InitializeResult | None = None
446477
self._discover_result: types.DiscoverResult | None = None
447478
self._discover_server_info: types.Implementation | None = None
@@ -1294,6 +1325,31 @@ def _absorb_tool_listing(self, result: types.ListToolsResult, *, complete: bool)
12941325
Idempotent: cached values are already post-filter, so the response cache can re-absorb a served listing.
12951326
`complete` (an uncursored single-page listing) prunes per-tool state down to the listing's tools.
12961327
"""
1328+
# Security pass: description injection scan + rug-pull detection.
1329+
# Runs for ALL protocol versions — rug-pull is not version-gated.
1330+
secure: list[types.Tool] = []
1331+
for tool in result.tools:
1332+
desc_hash = _tool_desc_hash(tool.description or "")
1333+
if tool.name in self._pinned_tool_desc_hashes:
1334+
if self._pinned_tool_desc_hashes[tool.name] != desc_hash:
1335+
logger.warning(
1336+
"security: tool %r description changed after approval — rug-pull detected; dropping",
1337+
tool.name,
1338+
)
1339+
self._x_mcp_header_maps.pop(tool.name, None)
1340+
continue
1341+
else:
1342+
if reason := _scan_tool_description(tool.description or ""):
1343+
logger.warning(
1344+
"security: tool %r description contains injection pattern (%s); dropping",
1345+
tool.name,
1346+
reason,
1347+
)
1348+
continue
1349+
self._pinned_tool_desc_hashes[tool.name] = desc_hash
1350+
secure.append(tool)
1351+
result.tools = secure
1352+
12971353
if self._negotiated_version in MODERN_PROTOCOL_VERSIONS:
12981354
# 2026-07-28: clients MUST drop tools whose x-mcp-header annotations are invalid.
12991355
kept: list[types.Tool] = []
@@ -1326,6 +1382,7 @@ def _absorb_tool_listing(self, result: types.ListToolsResult, *, complete: bool)
13261382
self._x_mcp_header_maps = {k: v for k, v in self._x_mcp_header_maps.items() if k in names}
13271383
self._tool_output_schemas = {k: v for k, v in self._tool_output_schemas.items() if k in names}
13281384
self._tool_output_validators = {k: v for k, v in self._tool_output_validators.items() if k in names}
1385+
self._pinned_tool_desc_hashes = {k: v for k, v in self._pinned_tool_desc_hashes.items() if k in names}
13291386

13301387
return result
13311388

‎src/mcp/server/transport_security.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,13 @@ class TransportSecurityMiddleware:
4444
"""Middleware to enforce DNS rebinding protection for MCP transport endpoints."""
4545

4646
def __init__(self, settings: TransportSecuritySettings | None = None):
47-
# If not specified, disable DNS rebinding protection by default for backwards compatibility
48-
self.settings = settings or TransportSecuritySettings(enable_dns_rebinding_protection=False)
47+
self.settings = settings or TransportSecuritySettings(enable_dns_rebinding_protection=True)
48+
if self.settings.enable_dns_rebinding_protection and not self.settings.allowed_hosts:
49+
logger.warning(
50+
"TransportSecurityMiddleware: DNS rebinding protection is enabled but allowed_hosts is empty — "
51+
"all requests will be rejected. Set allowed_hosts (e.g. ['localhost', '127.0.0.1:*']) "
52+
"or pass TransportSecuritySettings(enable_dns_rebinding_protection=False) to disable."
53+
)
4954

5055
def _validate_host(self, host: str | None) -> bool:
5156
"""Validate the Host header against allowed values."""

0 commit comments

Comments
 (0)