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
This commit is contained in:
Jeremiah Lowin 2026-03-15 14:22:01 -04:00 committed by GitHub
commit 8fdb3cc27c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 66 additions and 4 deletions

View file

@ -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":

View file

@ -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."""