fix: resolve dot-segments and percent-encoding in check_resource_allowed (#3303)

src/mcp/shared/auth_utils.py

@@ -1,7 +1,8 @@
 """Utilities for OAuth 2.0 Resource Indicators (RFC 8707) and PKCE (RFC 7636)."""
 
+import posixpath
 import time
-from urllib.parse import urlparse, urlsplit, urlunsplit
+from urllib.parse import unquote, urlparse, urlsplit, urlunsplit
 
 from pydantic import AnyUrl, HttpUrl
 
@@ -28,6 +29,18 @@ def resource_url_from_server_url(url: str | HttpUrl | AnyUrl) -> str:
     return canonical
 
 
+def _normalize_path(path: str) -> str:
+    """Percent-decode (single pass, per RFC 3986) and resolve "."/".." segments.
+
+    Anchoring at "/" keeps ".." from escaping above root. "%2f" decodes to "/"
+    and is treated as a separator (conservative for an authorization check).
+    """
+    decoded = unquote(path)
+    if not decoded:
+        return "/"
+    return posixpath.normpath("/" + decoded.lstrip("/"))
+
+
 def check_resource_allowed(requested_resource: str, configured_resource: str) -> bool:
     """Check if a requested resource URL matches a configured resource URL.
 
@@ -51,10 +64,12 @@ def check_resource_allowed(requested_resource: str, configured_resource: str) ->
     if requested.scheme.lower() != configured.scheme.lower() or requested.netloc.lower() != configured.netloc.lower():
         return False
 
+    # Resolve dot-segments/encoding so "/api/../admin" can't pass as "/api".
+    requested_path = _normalize_path(requested.path)
+    configured_path = _normalize_path(configured.path)
+
     # Normalize trailing slashes before comparison so that
     # "/foo" and "/foo/" are treated as equivalent.
-    requested_path = requested.path
-    configured_path = configured.path
     if not requested_path.endswith("/"):
         requested_path += "/"
     if not configured_path.endswith("/"):

tests/shared/test_auth_utils.py

@@ -121,3 +121,33 @@ def test_check_resource_allowed_empty_paths():
     assert check_resource_allowed("https://example.com", "https://example.com") is True
     assert check_resource_allowed("https://example.com/", "https://example.com") is True
     assert check_resource_allowed("https://example.com/api", "https://example.com") is True
+
+
+def test_check_resource_allowed_rejects_dot_segment_traversal():
+    """Traversal like /api/../admin resolves to /admin and must not match."""
+    assert check_resource_allowed("https://example.com/api/../admin", "https://example.com/api") is False
+    assert check_resource_allowed("https://example.com/api/../../etc", "https://example.com/api") is False
+    assert check_resource_allowed("https://example.com/api/./../admin", "https://example.com/api") is False
+
+
+def test_check_resource_allowed_rejects_percent_encoded_traversal():
+    """Percent-encoded dot-segments must be decoded before matching."""
+    assert check_resource_allowed("https://example.com/api/%2e%2e/admin", "https://example.com/api") is False
+    assert check_resource_allowed("https://example.com/api/%2e%2e%2fadmin", "https://example.com/api") is False
+
+
+def test_check_resource_allowed_allows_harmless_dot_segments():
+    """Dot-segments that resolve back within the configured resource still match."""
+    assert check_resource_allowed("https://example.com/api/v1/../v1/users", "https://example.com/api") is True
+    assert check_resource_allowed("https://example.com/api/./v1", "https://example.com/api") is True
+
+
+def test_check_resource_allowed_decodes_a_single_pass():
+    """Single-pass decode (RFC 3986): double-encoded "%252e%252e" stays literal."""
+    assert check_resource_allowed("https://example.com/api/%252e%252e/x", "https://example.com/api") is True
+
+
+def test_check_resource_allowed_normalizes_configured_path_encoding():
+    """Both sides are normalized, so encoded and decoded paths compare equal."""
+    assert check_resource_allowed("https://example.com/a b/x", "https://example.com/a%20b") is True
+    assert check_resource_allowed("https://example.com/a%20b/x", "https://example.com/a b") is True