Reject dot-segments in redirect URI allowlist matching (#3963)

This commit is contained in:
Jeremiah Lowin 2026-04-17 17:49:25 -04:00 committed by GitHub
commit 1e67c53a17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 114 additions and 20 deletions

View file

@ -298,13 +298,13 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) and AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c) by requiring explicit user approval.
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy and AS-in-the-middle attacks](#confused-deputy-attacks) by requiring explicit user approval.
**`True` (default) — always prompt:**
Users see the consent screen on every authorization. Strongest protection against AS-in-the-middle attacks where a malicious MCP server redirects the victim's browser into a legitimate proxy and relies on a previously-remembered approval to silently complete the flow.
**`"remember"` — silent consent on return:**
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) still prompt, which preserves the mitigation against AS-in-the-middle while keeping the prompt-free UX for direct client-initiated flows. Weaker than `True` — use only if the prompt friction is a problem.
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) fall back to the prompt. `Sec-Fetch-Site` is a browser-level heuristic rather than a protocol guarantee: an attacker who finds a way to initiate a non-cross-site navigation (XSS on a sibling origin, a same-site redirect chain, etc.) can reach the silent-consent path. `True` does not depend on this signal. See [Confused Deputy Attacks](#confused-deputy-attacks) for the underlying attack class.
**`"external"` — delegate to upstream:**
Skip the built-in consent page; consent is collected by the upstream IdP or a custom login page referenced via `upstream_authorization_endpoint`. No security warning is logged.
@ -651,7 +651,7 @@ The OAuth proxy works by bridging DCR clients to traditional auth providers, whi
FastMCP's OAuth proxy defends against confused deputy attacks with two layers of protection:
**Consent screen.** Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. Once you approve a client, it's remembered so you don't see the consent page again for that client. The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering.
**Consent screen.** Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. By default (`require_authorization_consent=True`), the page is shown on every flow, which is the strongest protection. Setting `require_authorization_consent="remember"` approves previously-approved `(client_id, redirect_uri)` pairs silently on return visits, trading some protection for UX (see below). The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering.
![](/assets/images/oauth-proxy-consent-screen.png)
@ -659,6 +659,12 @@ The consent page automatically displays your server's name, icon, and website UR
**Browser-session binding.** When you approve consent (or when a previously-approved client auto-approves), the proxy sets a cryptographically signed cookie that binds your browser session to the authorization flow. When the identity provider redirects back to the proxy's callback, the proxy verifies that this cookie is present and matches the expected transaction. A different browser — such as a victim who was sent the authorization URL by an attacker — won't have this cookie, and the callback will be rejected with a 403 error. This prevents the attack even when the identity provider skips the consent page for previously-authorized applications.
#### AS-in-the-middle variant
A related attack works even with browser-session binding in place: a malicious MCP server advertises its own authorization server, which redirects the victim's browser into the legitimate proxy's `/authorize` endpoint. Because the victim's browser carries both the prior-approval cookie and the newly-issued session-binding cookie throughout, both layers pass. The defense is the consent prompt itself: if consent is shown (`require_authorization_consent=True`), the victim sees the benign MCP server's name on the consent page — which doesn't match the malicious server they thought they were connecting to — and can deny.
`require_authorization_consent="remember"` adds a `Sec-Fetch-Site` check to keep this path safe for legitimate return flows (the attack navigation lands as `cross-site` and falls back to the prompt), but this is a browser-level heuristic. For the strongest defense, leave `require_authorization_consent=True`.
**Learn more:**
- [MCP Security Best Practices](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) - Official specification guidance
- [Confused Deputy Attacks Explained](https://den.dev/blog/mcp-confused-deputy-api-management/) - Detailed walkthrough by Den Delimarsky

View file

@ -288,13 +288,13 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) and AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c) by requiring explicit user approval.
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy and AS-in-the-middle attacks](#confused-deputy-attacks) by requiring explicit user approval.
**`True` (default) — always prompt:**
Users see the consent screen on every authorization. Strongest protection against AS-in-the-middle attacks where a malicious MCP server redirects the victim's browser into a legitimate proxy and relies on a previously-remembered approval to silently complete the flow.
**`"remember"` — silent consent on return:**
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) still prompt, which preserves the mitigation against AS-in-the-middle while keeping the prompt-free UX for direct client-initiated flows. Weaker than `True` — use only if the prompt friction is a problem.
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) fall back to the prompt. `Sec-Fetch-Site` is a browser-level heuristic rather than a protocol guarantee: an attacker who finds a way to initiate a non-cross-site navigation (XSS on a sibling origin, a same-site redirect chain, etc.) can reach the silent-consent path. `True` does not depend on this signal. See [Confused Deputy Attacks](#confused-deputy-attacks) for the underlying attack class.
**`"external"` — delegate to upstream:**
Skip the built-in consent page; consent is collected by the upstream IdP or a custom login page referenced via `upstream_authorization_endpoint`. No security warning is logged.
@ -566,13 +566,17 @@ The OAuth proxy works by bridging DCR clients to traditional auth providers, whi
#### Mitigation
FastMCP's OAuth proxy requires you to explicitly consent whenever any new or unrecognized client attempts to connect to your server. Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. Once you approve a client, it's remembered so you don't see the consent page again for that client. The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering.
FastMCP's OAuth proxy requires you to explicitly consent whenever a client attempts to connect to your server. Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. By default (`require_authorization_consent=True`), the page is shown on every flow, which is the strongest protection. Setting `require_authorization_consent="remember"` approves previously-approved `(client_id, redirect_uri)` pairs silently on return visits, trading some protection for UX (see below). The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering.
![](/assets/images/oauth-proxy-consent-screen.png)
The consent page automatically displays your server's name, icon, and website URL, if available. These visual identifiers help users confirm they're authorizing the correct server.
#### AS-in-the-middle variant
A related attack works by positioning a malicious authorization server between an MCP client and a legitimate proxy: a malicious MCP server advertises its own authorization server, which redirects the victim's browser into the legitimate proxy's `/authorize` endpoint. Because the victim's browser carries the prior-approval cookie throughout, a `"remember"`-mode proxy would silently complete the flow. The defense is the consent prompt itself: if consent is shown (`require_authorization_consent=True`), the victim sees the benign MCP server's name on the consent page — which doesn't match the malicious server they thought they were connecting to — and can deny.
`require_authorization_consent="remember"` adds a `Sec-Fetch-Site` check to keep this path safe for legitimate return flows (the attack navigation lands as `cross-site` and falls back to the prompt), but this is a browser-level heuristic. For the strongest defense, leave `require_authorization_consent=True`.
**Learn more:**
- [MCP Security Best Practices](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) - Official specification guidance

View file

@ -331,10 +331,10 @@ class ConsentMixin:
# Silent consent only fires in "remember" mode, and only when the
# request arrived via a safe navigation context. AS-in-the-middle
# attacks (GHSA-6x8h-498w-gv8c) surface as cross-site redirects from
# a third-party origin into /authorize; forcing the HTML prompt in
# that case preserves the consent-screen mitigation without blocking
# legitimate client-initiated flows (Sec-Fetch-Site: none).
# attacks surface as cross-site redirects from a third-party origin
# into /authorize; forcing the HTML prompt in that case preserves
# the consent-screen mitigation without blocking legitimate
# client-initiated flows (Sec-Fetch-Site: none).
if self._require_authorization_consent == "remember":
sec_fetch_site = request.headers.get("Sec-Fetch-Site")
# Fail closed on missing header: legacy clients degrade to the

View file

@ -313,8 +313,8 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
If not provided, it will be derived from the upstream client secret using HKDF.
require_authorization_consent: Consent screen behavior (default True).
- True: always show the consent screen before redirecting to the
upstream IdP. Strongest protection against AS-in-the-middle attacks
(GHSA-6x8h-498w-gv8c).
upstream IdP. Strongest protection against AS-in-the-middle
attacks.
- "remember": show the consent screen the first time, then silently
approve subsequent authorizations for the same (client_id,
redirect_uri) in the same browser. Cross-site navigations are
@ -417,7 +417,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
"Consent screen in 'remember' mode: silent consent on return visits "
"for previously-approved clients (with Sec-Fetch-Site gating). "
"Set require_authorization_consent=True for strongest protection "
"against AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c)."
"against AS-in-the-middle attacks."
)
elif not require_authorization_consent:
logger.warning(

View file

@ -5,7 +5,7 @@ protecting against userinfo-based bypass attacks like http://localhost@evil.com.
"""
import fnmatch
from urllib.parse import urlparse
from urllib.parse import unquote, urlparse
from pydantic import AnyUrl
@ -106,6 +106,21 @@ def _match_port(
return uri_effective == pattern_effective
def _has_dot_segments(path: str) -> bool:
"""Return True if a URI path contains `.` or `..` segments.
Browsers collapse dot-segments when resolving a 302 Location per RFC
3986 §5.2.4. Allowing them through the allowlist lets an attacker craft
a URI that passes pattern matching but lands on a different path after
redirect. Checks both the raw path and its percent-decoded form so that
encoded variants like `/foo/%2e%2e/bar` are rejected.
"""
for candidate in (path, unquote(path)):
if any(seg in (".", "..") for seg in candidate.split("/")):
return True
return False
def _match_path(uri_path: str, pattern_path: str) -> bool:
"""Match path component using fnmatch for wildcard support.
@ -163,6 +178,15 @@ def matches_allowed_pattern(uri: str, pattern: str) -> bool:
if uri_parsed.username is not None or uri_parsed.password is not None:
return False
# SECURITY: Reject URIs with dot-segments in the path.
# fnmatch's `*` matches across `/`, so a pattern like `/oauth/callback/*`
# would accept `/oauth/callback/../../steal`; a browser receiving that in
# a 302 Location resolves the dot-segments and lands at `/steal`, outside
# the intended allowlist prefix. Reject at validation time so the stored
# redirect_uri cannot later be emitted verbatim in a redirect.
if _has_dot_segments(uri_parsed.path):
return False
# Scheme must match exactly
if uri_parsed.scheme.lower() != pattern_parsed.scheme.lower():
return False

View file

@ -733,9 +733,9 @@ class TestConsentSecurity:
class TestConsentModes:
"""Tests for the three consent modes: True (always), 'remember' (silent), 'external'.
Covers the default-behavior change in GHSA-6x8h-498w-gv8c: True no longer
silently approves return visits, and 'remember' mode gates silent consent
on Sec-Fetch-Site to block AS-in-the-middle attacks.
Covers the default behavior where True no longer silently approves return
visits, and 'remember' mode gates silent consent on Sec-Fetch-Site to
block AS-in-the-middle attacks.
"""
async def test_default_mode_does_not_set_approval_cookie(self, oauth_proxy_https):
@ -818,9 +818,9 @@ class TestConsentModes:
):
"""Sec-Fetch-Site=cross-site must skip silent consent even with valid cookie.
Blocks the GHSA-6x8h-498w-gv8c AS-in-the-middle scenario: attacker
redirects the victim's browser to /authorize from a third-party origin,
which browsers mark cross-site. Legit local-host flows are Sec-Fetch-Site=none.
Blocks the AS-in-the-middle scenario: attacker redirects the victim's
browser to /authorize from a third-party origin, which browsers mark
cross-site. Legit local-host flows are Sec-Fetch-Site=none.
"""
client_id = "client-cross"
redirect = "http://localhost:7003/callback"

View file

@ -168,6 +168,66 @@ class TestSecurityBypass:
assert not matches_allowed_pattern("http://example.com:3000/callback", pattern)
class TestDotSegmentBypass:
"""Tests for dot-segment bypass of path allowlists.
A pattern like `/oauth/callback/*` would otherwise match
`/oauth/callback/../../steal` via `fnmatch`, because `*` matches across
`/`. Browsers resolve the dot-segments on redirect, landing at a path
outside the allowlist prefix. The validator rejects dot-segments (raw
and percent-encoded) up front.
"""
def test_traversal_segments_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
assert not matches_allowed_pattern(
"https://app.example.com/oauth/callback/../../steal", pattern
)
def test_percent_encoded_traversal_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
for encoded in ("%2e%2e", "%2E%2E", "%2e%2E"):
assert not matches_allowed_pattern(
f"https://app.example.com/oauth/callback/{encoded}/{encoded}/steal",
pattern,
)
def test_single_dot_segment_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
assert not matches_allowed_pattern(
"https://app.example.com/oauth/callback/./foo", pattern
)
def test_trailing_dotdot_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
assert not matches_allowed_pattern(
"https://app.example.com/oauth/callback/foo/..", pattern
)
def test_percent_encoded_single_dot_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
assert not matches_allowed_pattern(
"https://app.example.com/oauth/callback/%2e/foo", pattern
)
def test_mixed_raw_and_encoded_rejected(self):
pattern = "https://app.example.com/oauth/callback/*"
assert not matches_allowed_pattern(
"https://app.example.com/oauth/callback/..%2fsteal", pattern
)
def test_legitimate_nested_paths_still_match(self):
"""Paths that happen to contain dots in segment names must still match."""
pattern = "https://app.example.com/oauth/callback/*"
for uri in (
"https://app.example.com/oauth/callback/foo",
"https://app.example.com/oauth/callback/deeply/nested/path",
"https://app.example.com/oauth/callback/file.ext",
"https://app.example.com/oauth/callback/v1.2.3/item",
):
assert matches_allowed_pattern(uri, pattern), uri
class TestLoopbackPortMatching:
"""Test RFC 8252 §7.3: loopback URIs with no port in pattern match any port."""