fix: reject URL-encoded path separators in resource template parameters#2353
Open
sebastiondev wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
Conversation
ResourceTemplate.matches() URL-decodes extracted parameters but did not
re-validate that the decoded values still satisfy the [^/]+ segment
constraint. An attacker could send a URI like:
files://..%2F..%2Fetc%2Fpasswd
The encoded %2F passes the regex match on the raw URI, but after
unquote() it becomes ../../etc/passwd — a path traversal payload that
is then passed directly to the template function via fn(**params).
The fix re-checks every decoded parameter value against the original
[^/]+ pattern and returns None (no match) if any value now contains a
forward slash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vulnerability Summary
CWE-22: Improper Limitation of a Pathname to a Restricted Directory ("Path Traversal")
Severity: High
Affected file:
src/mcp/server/mcpserver/resources/templates.py—ResourceTemplate.matches()Data Flow
resources/readrequest with a crafted URI (e.g.file://documents/..%2F..%2F..%2Fetc%2Fpasswd).MCPServer._handle_read_resource()routes the request toResourceManager.get_resource()(line 94,resource_manager.py).ResourceTemplate.matches()converts the URI template to a regex where each{param}becomes(?P<param>[^/]+)...%2F..%2F..%2Fetc%2Fpasswdcontains no literal/, so[^/]+succeeds.unquote()then decodes it to../../../etc/passwd, which is returned as the parameter value.Exploit Sketch
Before fix:
name="../../../secrets/api_keys.json"→ resolves to/secrets/api_keys.jsonAfter fix:
matches()returnsNone→ request is rejectedFix Description
The fix adds a post-decode validation step in
ResourceTemplate.matches(). After URL-decoding extracted parameter values, each value is re-validated against the same[^/]+segment constraint that was applied to the encoded form. If a decoded value contains a/character (which could only come from an encoded%2F),matches()returnsNone.Rationale
matches()is the only code path that extracts template parameters from URIs in the MCP server framework. Fixing it here provides defense-in-depth for all resource handlers./is a reserved character;%2Fin a path segment should not be treated as equivalent to a literal/for matching purposes.%2eencoding) and similar issues fixed in Flask/Werkzeug.Change Details
Test Results Summary
Tested the following scenarios to confirm correctness:
file://docs/readme.txt{"name": "readme.txt"}%2Ffile://docs/..%2F..%2Fetc%2FpasswdNone(rejected)%2f(lowercase)file://docs/..%2f..%2fetc%2fpasswdNone(rejected)%252Ffile://docs/..%252F..%252Fetc{"name": "..%2F..%2Fetc"}(single decode, no/)file://docs/hello%20world.txt{"name": "hello world.txt"}file://{org}/{repo}with%2Fin one paramNone(rejected)file://docs/%2FNone(rejected)file://other/pathagainstfile://docs/{name}NoneDisprove Analysis
We systematically attempted to disprove this finding across multiple dimensions:
Authentication Check
No authentication on
ResourceTemplate.matches()orread_resource. Auth is opt-in viaMCPServer(auth=...). Default servers run without auth. Theresources/readhandler is accessible to any connected MCP client.Network Check
Default transport is
stdio(local process). HTTP transports default to127.0.0.1with DNS rebinding protection. However, servers can be configured to bind to0.0.0.0or deployed over HTTP — the README shows streamable HTTP examples suggesting production HTTP use is expected.Caller Trace
matches()is called from exactly one place:ResourceManager.get_resource()→MCPServer.read_resource()→_handle_read_resource(). The URI comes directly fromparams.uri(client-supplied). Attacker-controlled data reaches this code path.Prior Validation
Zero validation existed on decoded parameter values before this fix. Pydantic
validate_callonly enforces type (str), not content.Existing Mitigations
stdiotransport reduces remote attack surfaceSimilar Code Paths
The low-level server API does not use
ResourceTemplate— it delegates to user callbacks directly. This is not a parallel vulnerability because low-level users don't use templates and are responsible for their own parsing. No other instances of template parameter extraction without post-decode validation exist.Known Limitation
The fix does not block
%5C→\(Windows backslash path separator). On Windows,..\..\secretis a valid traversal. This is a separate concern given the URI spec uses/exclusively, and could be addressed in a follow-up if desired.Verdict
Confirmed valid with high confidence. The vulnerability is clearly demonstrable, the fix is correct and minimal, and it addresses the primary attack vector at the single chokepoint for template parameter extraction.
Preconditions for Exploitation
This fix was prepared through a 4-stage review process including vulnerability identification, fix development, testing, and adversarial disprove analysis.