Skip to content

Security: OAuth/session findings from code review #626

Description

@tkaufmann

Context

While reviewing the codebase (v0.65.0) for a BasicAuth deployment, I noticed several security issues in the OAuth/session paths. I'm submitting PRs for the input validation issues I encountered (XML injection, path traversal, SSRF, etc.), but the findings below are in code paths I'm not modifying, so I'm reporting them here.

Findings

1. JWT decoded without signature verification (Critical)

Files: auth/token_exchange.py:305,340, auth/token_broker.py:527, auth/provisioning_decorator.py:82,165, auth/token_utils.py:49, auth/browser_oauth_routes.py:373, auth/oauth_routes.py:~595

8+ locations use jwt.decode(token, options={"verify_signature": False}). In _validate_flow1_token() (token_exchange.py), audience/expiry/subject checks run on unverified claims. Code comments confirm awareness ("In production, should verify signature").

Impact: An attacker can forge arbitrary JWT claims. In token exchange mode, a self-crafted JWT with aud: "mcp-server" enables full identity takeover.

CWE-347 (Improper Verification of Cryptographic Signature)

2. Predictable session cookie value

Files: auth/browser_oauth_routes.py:448-453, auth/session_backend.py:80-81

The mcp_session cookie is set to the raw user_id (the sub claim from the ID token). No HMAC, no signature, no cryptographically random session token. The user_id is typically a predictable UUID from the IdP.

Impact: If an attacker learns another user's user_id (exposed in debug logs), they can forge the cookie and access the admin UI and stored refresh tokens.

CWE-287, CWE-384

3. LLM-controllable user_id parameter in OAuth tools

File: server/oauth_tools.py:588-640

Tools provision_nextcloud_access, revoke_nextcloud_access, check_provisioning_status, and check_logged_in accept an optional user_id parameter. An LLM or direct MCP client can supply an arbitrary user ID.

Impact: Cross-user attacks: revoking another user's Nextcloud access (DoS), querying other users' provisioning status (information disclosure).

Fix: Remove user_id from public tool signatures; always extract from the authenticated token.

CWE-639 (Authorization Bypass Through User-Controlled Key)

4. Logout does not revoke refresh token

File: auth/browser_oauth_routes.py:459-479

The logout handler only deletes the cookie. The refresh token remains in the database. This is marked as a TODO in the code.

Impact: After logout, the session is not invalidated server-side. Refresh tokens remain valid for up to 90 days.

CWE-613 (Insufficient Session Expiration)

5. Hardcoded encryption keys in docker-compose.yml

File: docker-compose.yml:151,190,271,314

Four Fernet encryption keys are hardcoded in the committed docker-compose.yml.

Impact: Anyone with repo access can decrypt all stored tokens.

Fix: Use placeholder values with comments, or move to .env.example.

Notes

  • Findings 1-4 only affect the OAuth code path. BasicAuth-only deployments are not impacted.
  • I'm happy to help with fixes if that's useful — just didn't want to scope-creep my existing PRs.

Metadata

Metadata

Assignees

Labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions