From 8fdb3cc27c92a0a29aaa3edd0577054b15e3eb52 Mon Sep 17 00:00:00 2001
From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
Date: Sun, 15 Mar 2026 14:22:01 -0400
Subject: [PATCH] fix: CSRF double-submit cookie check in consent flow (#3519)

* Upgrade examples/testing_demo lockfile, drops diskcache (CVE-2025-69872)

* fix: add CSRF double-submit cookie check to consent flow (GHSA-rww4-4w9c-7733)

* fix: preserve CSRF state across concurrent flows, fix test isolation

* fix: reject non-__Host consent-state cookie on HTTPS
---
 .../server/auth/oauth_proxy/consent.py        | 31 +++++++++++++--
 tests/server/auth/test_oauth_consent_flow.py  | 39 +++++++++++++++++++
 2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/src/fastmcp/server/auth/oauth_proxy/consent.py b/src/fastmcp/server/auth/oauth_proxy/consent.py
index 597b8f87e..b2ad20642 100644
--- a/src/fastmcp/server/auth/oauth_proxy/consent.py
+++ b/src/fastmcp/server/auth/oauth_proxy/consent.py
@@ -110,9 +110,13 @@ class ConsentMixin:
         self: OAuthProxy, request: Request, base_name: str
     ) -> list[str]:
         """Decode and verify a signed base64-encoded JSON list from cookie. Returns [] if missing/invalid."""
-        # Prefer secure name, but also check non-secure variant for dev
         secure_name = self._cookie_name(base_name)
-        raw = request.cookies.get(secure_name) or request.cookies.get(f"__{base_name}")
+        raw = request.cookies.get(secure_name)
+        # Only fall back to the non-__Host- name over plain HTTP. On HTTPS,
+        # __Host- enforces host-only scope; accepting the weaker name would
+        # let a sibling-subdomain attacker inject a domain-scoped cookie.
+        if not raw and not self._is_https:
+            raw = request.cookies.get(f"__{base_name}")
         if not raw:
             return []
         try:
@@ -397,11 +401,13 @@ class ConsentMixin:
             cimd_domain=cimd_domain,
         )
         response = create_secure_html_response(html)
-        # Store CSRF in cookie with short lifetime
+        # Merge new CSRF token with any existing ones (supports concurrent flows)
+        existing_tokens = self._decode_list_cookie(request, "MCP_CONSENT_STATE")
+        existing_tokens.append(csrf_token)
         self._set_list_cookie(
             response,
             "MCP_CONSENT_STATE",
-            self._encode_list_cookie([csrf_token]),
+            self._encode_list_cookie(existing_tokens),
             max_age=15 * 60,
         )
         return response
@@ -435,6 +441,23 @@ class ConsentMixin:
                 "<h1>Error</h1><p>Invalid or expired consent token</p>", status_code=400
             )
 
+        # Double-submit CSRF check: verify the form token matches the cookie.
+        # Without this, an attacker who knows their own tx_id/csrf_token can
+        # CSRF the victim's browser into approving consent, bypassing the
+        # consent binding cookie protection.
+        cookie_csrf_tokens = self._decode_list_cookie(request, "MCP_CONSENT_STATE")
+        if csrf_token not in cookie_csrf_tokens:
+            logger.warning(
+                "CSRF double-submit check failed for transaction %s "
+                "(possible cross-site consent forgery)",
+                txn_id,
+            )
+            return create_secure_html_response(
+                "<h1>Error</h1><p>Authorization session mismatch. "
+                "Please try authenticating again.</p>",
+                status_code=403,
+            )
+
         client_key = self._make_client_key(txn["client_id"], txn["client_redirect_uri"])
 
         if action == "approve":
diff --git a/tests/server/auth/test_oauth_consent_flow.py b/tests/server/auth/test_oauth_consent_flow.py
index 7a65df297..025eb3452 100644
--- a/tests/server/auth/test_oauth_consent_flow.py
+++ b/tests/server/auth/test_oauth_consent_flow.py
@@ -462,6 +462,45 @@ class TestCSRFProtection:
             )
 
 
+class TestCSRFDoubleSubmit:
+    """Tests for CSRF double-submit cookie validation (GHSA-rww4-4w9c-7733 bypass)."""
+
+    async def test_consent_rejected_without_csrf_cookie(self, oauth_proxy_with_storage):
+        """Submitting a valid CSRF token without the matching cookie should be rejected.
+
+        This prevents an attacker from using their own tx_id/csrf_token to CSRF
+        the victim's browser into approving consent.
+        """
+        txn_id, _ = await _start_flow(
+            oauth_proxy_with_storage,
+            "csrf-double-submit-client",
+            "http://localhost:9090/callback",
+        )
+
+        app = Starlette(routes=oauth_proxy_with_storage.get_routes())
+        with TestClient(app) as test_client:
+            # Visit consent page to populate the transaction with a CSRF token
+            consent_resp = test_client.get(f"/consent?txn_id={txn_id}")
+            assert consent_resp.status_code == 200
+            csrf_token = _extract_csrf(consent_resp.text)
+            assert csrf_token
+
+        # Simulate the attack: use a FRESH client (no cookies from the consent
+        # page) to submit the form with a valid CSRF token — as if the attacker
+        # tricked the victim's browser into POSTing their tx_id/csrf_token.
+        with TestClient(app) as attacker_client:
+            response = attacker_client.post(
+                "/consent",
+                data={
+                    "action": "approve",
+                    "txn_id": txn_id,
+                    "csrf_token": csrf_token,
+                },
+                follow_redirects=False,
+            )
+            assert response.status_code == 403
+
+
 class TestStoragePersistence:
     """Tests for state persistence across storage backends."""