Skip to content

Commit 4475d77

Browse files
committed
fix: address security review feedback
- Disable follow_redirects to prevent SSRF bypass via open redirects - Add explicit IP obfuscation detection (decimal/octal/hex formats) - Fix SSL parsing to be fail-secure (only 'false' disables verification) - Clean up test headers (remove enterprise roleplay language) - Add comprehensive tests for IP obfuscation parsing
1 parent a481f42 commit 4475d77

2 files changed

Lines changed: 139 additions & 14 deletions

File tree

src/fetch/src/mcp_server_fetch/server.py

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030

3131
# SSL Certificate Verification Configuration
3232
# Set MCP_FETCH_SSL_VERIFY=false to disable SSL verification for internal/self-signed certificates
33-
SSL_VERIFY = os.getenv("MCP_FETCH_SSL_VERIFY", "true").lower() == "true"
33+
# NOTE: Only explicit "false" disables verification; any other value (including typos) keeps it enabled.
34+
SSL_VERIFY = os.getenv("MCP_FETCH_SSL_VERIFY", "true").lower() != "false"
3435

3536
# SSRF Protection Configuration
3637
# Set MCP_FETCH_ALLOW_PRIVATE_IPS=true to allow fetching from private/internal networks
@@ -72,6 +73,68 @@
7273
])
7374

7475

76+
def _parse_obfuscated_ip(hostname: str) -> str | None:
77+
"""
78+
Detect and decode obfuscated IP address formats.
79+
80+
Attackers may use alternative IP representations to bypass SSRF filters:
81+
- Decimal: 2130706433 (= 127.0.0.1)
82+
- Octal: 0177.0.0.1 (= 127.0.0.1)
83+
- Hex: 0x7f000001 (= 127.0.0.1)
84+
- Mixed: 0x7f.0.0.1 (= 127.0.0.1)
85+
86+
Returns the normalized IP string if detected, None otherwise.
87+
"""
88+
hostname = hostname.strip()
89+
90+
# Try decimal integer format (e.g., 2130706433 = 127.0.0.1)
91+
try:
92+
if hostname.isdigit():
93+
ip_int = int(hostname)
94+
if 0 <= ip_int <= 0xFFFFFFFF: # Valid 32-bit range
95+
# Convert to dotted decimal
96+
return str(ipaddress.IPv4Address(ip_int))
97+
except (ValueError, ipaddress.AddressValueError):
98+
pass
99+
100+
# Try hex format (e.g., 0x7f000001 = 127.0.0.1)
101+
try:
102+
if hostname.lower().startswith("0x") and "." not in hostname:
103+
ip_int = int(hostname, 16)
104+
if 0 <= ip_int <= 0xFFFFFFFF:
105+
return str(ipaddress.IPv4Address(ip_int))
106+
except (ValueError, ipaddress.AddressValueError):
107+
pass
108+
109+
# Try octal/hex dotted format (e.g., 0177.0.0.1 or 0x7f.0.0.1)
110+
# Only return if there's actual obfuscation (hex prefix or leading zeros)
111+
if "." in hostname:
112+
parts = hostname.split(".")
113+
if len(parts) == 4:
114+
try:
115+
octets = []
116+
has_obfuscation = False
117+
for part in parts:
118+
part = part.strip()
119+
if part.lower().startswith("0x"):
120+
octets.append(int(part, 16))
121+
has_obfuscation = True
122+
elif part.startswith("0") and len(part) > 1 and part.isdigit():
123+
# Octal format (leading zero with more digits)
124+
octets.append(int(part, 8))
125+
has_obfuscation = True
126+
else:
127+
octets.append(int(part))
128+
129+
# Only return if we detected obfuscation AND result is valid
130+
if has_obfuscation and all(0 <= o <= 255 for o in octets):
131+
return f"{octets[0]}.{octets[1]}.{octets[2]}.{octets[3]}"
132+
except ValueError:
133+
pass
134+
135+
return None
136+
137+
75138
def _is_ip_private_or_reserved(ip_str: str) -> bool:
76139
"""
77140
Check if an IP address is private, reserved, loopback, or link-local.
@@ -202,9 +265,22 @@ def validate_url_for_ssrf(url: str) -> None:
202265
f"This hostname is associated with internal services.",
203266
))
204267

205-
# Try to parse hostname as IP address directly (handles obfuscation)
268+
# Check for obfuscated IP addresses (decimal, octal, hex encoding)
269+
# Python's ipaddress module does NOT parse these from strings, so we handle them explicitly
270+
obfuscated_ip = _parse_obfuscated_ip(hostname)
271+
if obfuscated_ip:
272+
if _is_ip_private_or_reserved(obfuscated_ip):
273+
if not ALLOW_PRIVATE_IPS:
274+
raise McpError(ErrorData(
275+
code=INVALID_PARAMS,
276+
message=f"Access to obfuscated private IP address '{hostname}' "
277+
f"(decoded: {obfuscated_ip}) is blocked. "
278+
f"Set MCP_FETCH_ALLOW_PRIVATE_IPS=true to allow internal network access.",
279+
))
280+
return
281+
282+
# Try to parse hostname as standard IP address
206283
try:
207-
# This handles decimal (2130706433), octal (0177.0.0.1), hex (0x7f.0.0.1)
208284
ip = ipaddress.ip_address(hostname)
209285
if _is_ip_private_or_reserved(str(ip)):
210286
if not ALLOW_PRIVATE_IPS:
@@ -305,7 +381,7 @@ async def check_may_autonomously_fetch_url(url: str, user_agent: str, proxy_url:
305381
try:
306382
response = await client.get(
307383
robot_txt_url,
308-
follow_redirects=True,
384+
follow_redirects=False,
309385
headers={"User-Agent": user_agent},
310386
timeout=30,
311387
)
@@ -383,7 +459,7 @@ async def fetch_url(
383459
try:
384460
response = await client.get(
385461
url,
386-
follow_redirects=True,
462+
follow_redirects=False,
387463
headers={"User-Agent": user_agent},
388464
timeout=30,
389465
)

src/fetch/tests/test_security.py

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
"""
2-
Enterprise Security Test Suite for MCP Fetch Server
3-
====================================================
2+
Security regression tests for fetch server.
43
54
This test suite validates the security controls implemented in server.py:
65
1. SSL Certificate Verification Toggle
76
2. SSRF (Server-Side Request Forgery) Protection
87
3. Payload Size Limits
98
4. URL Scheme Validation
10-
11-
Author: Enterprise Security Team
12-
Date: 2025-12-31
13-
Coverage Target: 95%+
149
"""
1510

1611
import os
@@ -27,6 +22,7 @@
2722
_is_hostname_blocked,
2823
_is_hostname_whitelisted,
2924
_normalize_hostname,
25+
_parse_obfuscated_ip,
3026
fetch_url,
3127
extract_content_from_html,
3228
BLOCKED_HOSTNAMES,
@@ -126,8 +122,8 @@ async def test_ssl_verify_invalid_value_defaults_to_false(self, reset_env):
126122
import mcp_server_fetch.server as server_module
127123
importlib.reload(server_module)
128124

129-
# 'invalid'.lower() == 'true' is False
130-
assert server_module.SSL_VERIFY is False
125+
# Fail-secure: only explicit "false" disables SSL verification
126+
assert server_module.SSL_VERIFY is True
131127

132128
@pytest.mark.asyncio
133129
async def test_ssl_disabled_allows_self_signed(self, reset_env):
@@ -166,7 +162,60 @@ async def test_ssl_disabled_allows_self_signed(self, reset_env):
166162

167163

168164
# =============================================================================
169-
# 2. SSRF PROTECTION TESTS
165+
# 2. IP OBFUSCATION PARSING TESTS
166+
# =============================================================================
167+
168+
class TestIPObfuscationParsing:
169+
"""Test suite for IP obfuscation detection and parsing."""
170+
171+
@pytest.mark.parametrize("obfuscated,expected", [
172+
# Decimal encoding (127.0.0.1 = 2130706433)
173+
("2130706433", "127.0.0.1"),
174+
# Decimal encoding (169.254.169.254 = 2852039166)
175+
("2852039166", "169.254.169.254"),
176+
# Hex encoding
177+
("0x7f000001", "127.0.0.1"),
178+
("0x7F000001", "127.0.0.1"), # uppercase
179+
# Octal dotted format
180+
("0177.0.0.1", "127.0.0.1"),
181+
("0177.0.0.01", "127.0.0.1"),
182+
# Hex dotted format
183+
("0x7f.0.0.1", "127.0.0.1"),
184+
("0x7f.0x0.0x0.0x1", "127.0.0.1"),
185+
])
186+
def test_parses_obfuscated_ips(self, obfuscated, expected):
187+
"""Obfuscated IP formats should be correctly decoded."""
188+
result = _parse_obfuscated_ip(obfuscated)
189+
assert result == expected, f"Failed to parse {obfuscated}"
190+
191+
@pytest.mark.parametrize("normal_input", [
192+
"127.0.0.1", # Normal IP - not obfuscated
193+
"example.com", # Hostname
194+
"google.com",
195+
"192.168.1.1", # Normal private IP
196+
"", # Empty
197+
"not-an-ip",
198+
])
199+
def test_returns_none_for_normal_inputs(self, normal_input):
200+
"""Normal hostnames and IPs should return None (not obfuscated)."""
201+
result = _parse_obfuscated_ip(normal_input)
202+
assert result is None, f"Should not parse {normal_input} as obfuscated"
203+
204+
def test_blocks_obfuscated_loopback_via_validation(self):
205+
"""Obfuscated loopback should be blocked by validate_url_for_ssrf."""
206+
with pytest.raises(McpError) as exc_info:
207+
validate_url_for_ssrf("http://2130706433/")
208+
assert "obfuscated" in str(exc_info.value).lower() or "blocked" in str(exc_info.value).lower()
209+
210+
def test_blocks_obfuscated_metadata_via_validation(self):
211+
"""Obfuscated metadata IP should be blocked by validate_url_for_ssrf."""
212+
with pytest.raises(McpError) as exc_info:
213+
validate_url_for_ssrf("http://2852039166/") # 169.254.169.254
214+
assert "obfuscated" in str(exc_info.value).lower() or "blocked" in str(exc_info.value).lower()
215+
216+
217+
# =============================================================================
218+
# 3. SSRF PROTECTION TESTS
170219
# =============================================================================
171220

172221
class TestSSRFProtection:

0 commit comments

Comments
 (0)