Back to skill

Security audit

MCP OAuth

Security checks for vulnerabilities and agentic risk

Overview

The skill is not malicious, but its OAuth examples could generate weak login protection for an MCP server.

Install only if you treat it as a starting reference, not production-ready security code. Before using its output, require exact client and redirect URI registration, enforce PKCE verifier checks, use secure token storage with revocation and rotation, and add centralized authorization tests proving protected MCP tools cannot run without valid auth.

Vulnerability Patterns
  • Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
  • Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
  • Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
  • Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
  • Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally
Findings (4)

T09 · Insecure Skill Coding Practices

Error
Location
SKILL.md:134
Finding
Client PKCE Challenge Is Never Verified During Token Exchange<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 134-171 and 229-244 **Vulnerability Type**: Incomplete OAuth 2.0 PKCE implementation **Risk Level**: High ### Complete Code Snippet ```typescript const redirectUri = params.get('redirect_uri'); const state = params.get('state'); const codeChallenge = params.get('code_challenge'); if (!redirectUri || !state || !codeChallenge) { return NextResponse.json( { error: 'invalid_request', error_description: 'Missing required parameters' }, { status: 400 }, ); } // ... await redis.set(`session:${sessionId}`, JSON.stringify({ redirectUri, state, codeChallenge, upstreamVerifier, upstreamState: sessionId, }), { ex: 600 }); ``` The subsequent token exchange does not request or verify a PKCE verifier: ```typescript if (body.grant_type === 'authorization_code') { const userId = await redis.get(`auth_code:${body.code}`); if (!userId) return NextResponse.json({ error: 'invalid_grant' }, { status: 400 }); await redis.del(`auth_code:${body.code}`); const accessToken = crypto.randomBytes(16).toString('hex'); const refreshToken = crypto.randomBytes(16).toString('hex'); await redis.set(`mcp_token:${accessToken}`, userId, { ex: 86400 }); await redis.set(`refresh:${refreshToken}`, userId, { ex: 2592000 }); return NextResponse.json({ access_token: accessToken, token_type: 'Bearer', expires_in: 86400, refresh_token: refreshToken, }); } ``` ### Technical Analysis The authorization endpoint collects and stores the MCP client's `code_challenge`, but the generated authorization-code record contains only the user ID. The token endpoint neither requires a `code_verifier` nor calculates and compares its S256 digest with the original challenge. Consequently, possession of the authorization code alone is sufficient to obtain tokens. This defeats PKCE's primary security property: preventing an intercepted authorization code from being redeemed by a party that did not ...[truncated 1301 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Store an authorization-code record containing at least: - User identifier - `client_id` - Exact `redirect_uri` - `code_challenge` - `code_challenge_method` - Issuance and expiration timestamps - Require `code_verifier` at the token endpoint. - Enforce `code_challenge_method=S256`; do not silently permit plaintext PKCE. - Compute `BASE64URL(SHA256(code_verifier))` and compare it with the stored challenge using a timing-safe comparison. - Reject malformed verifiers and challenges according to PKCE length and character requirements. - Atomically consume the authorization code only when processing redemption, while ensuring failed attempts cannot create race conditions. - Issue tokens only after PKCE, client, redirect URI, expiration, and authorization-code checks all succeed. ]]>

T09 · Insecure Skill Coding Practices

Error
Location
SKILL.md:105
Finding
Dynamic Client Registration and Redirect URI Binding Are Not Enforced<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 105-118 and 134-160 **Vulnerability Type**: OAuth client and redirect URI validation failure **Risk Level**: High ### Complete Code Snippet The registration endpoint creates a client identifier but does not persist the client or its redirect URIs: ```typescript export async function POST(req: NextRequest) { const body = await req.json().catch(() => ({})); const clientId = crypto.randomBytes(16).toString('hex'); return NextResponse.json({ client_id: clientId, client_name: body.client_name || 'MCP Client', redirect_uris: body.redirect_uris || [], grant_types: ['authorization_code', 'refresh_token'], response_types: ['code'], token_endpoint_auth_method: 'none', }, { status: 201 }); } ``` The authorization endpoint does not require a registered `client_id` and only checks the redirect URI hostname: ```typescript const params = req.nextUrl.searchParams; const redirectUri = params.get('redirect_uri'); const state = params.get('state'); const codeChallenge = params.get('code_challenge'); if (!redirectUri || !state || !codeChallenge) { return NextResponse.json( { error: 'invalid_request', error_description: 'Missing required parameters' }, { status: 400 }, ); } // Validate redirect_uri — allow known MCP clients const url = new URL(redirectUri); const isAllowed = url.hostname === 'claude.ai' || url.hostname === 'claude.com' || url.hostname === 'api.smithery.ai' || url.hostname === 'localhost' || url.hostname === '127.0.0.1'; if (!isAllowed) { return NextResponse.json( { error: 'invalid_request', error_description: 'redirect_uri not allowed' }, { status: 400 }, ); } ``` ### Technical Analysis The dynamic registration endpoint returns a random client identifier but does not save the client record or validate its metadata. The authorization endpoint does not request or verify `client_id`, and it does not compare `redirect_uri` ...[truncated 1974 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Persist every registered client record in a durable store. - Validate registration metadata, including allowed schemes, URI syntax, grant types, response types, and token authentication method. - Require `client_id` at the authorization endpoint and reject unknown or disabled clients. - Compare the supplied redirect URI against an exact URI registered to that client, including scheme, host, port, path, and query rules. - Do not use hostname-only matching as a substitute for registered redirect URI comparison. - Restrict localhost callbacks to explicitly registered loopback URI patterns and follow the native-application loopback guidance applicable to the client type. - Bind the authorization code to `client_id` and exact `redirect_uri`. - Require the same client and redirect URI values during token redemption. - Apply PKCE independently; redirect URI validation does not replace PKCE. ]]>

T09 · Insecure Skill Coding Practices

Error
Location
SKILL.md:286
Finding
Protected MCP Tool Execution Is Not Enforced by the Authentication Wrapper<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 286-294 **Vulnerability Type**: Missing centralized authorization enforcement **Risk Level**: High ### Complete Code Snippet ```typescript // required: false allows initialize/tools/list without auth // Tools check auth themselves via extra.authInfo const handler = withMcpAuth(mcpHandler, verifyToken, { required: false, resourceUrl: SITE_URL, }); ``` The accompanying guidance states: ```text Setting `required: false` is important — it allows MCP clients to discover tools without authenticating first. Auth is enforced at the tool level when the tool tries to access user data. ``` ### Technical Analysis The wrapper is explicitly configured with `required: false`, so authentication is not a mandatory precondition for requests reaching the MCP handler. The documentation delegates access control to each individual tool through `extra.authInfo`, but it does not provide or enforce a centralized per-tool authorization guard. This creates a fail-open design. Any protected tool that omits the manual check, performs it incorrectly, or introduces a new execution path without the check becomes anonymously callable. Listing tools without authentication may be intentional, but that does not require allowing unauthenticated execution through the same unguarded path. ### Attack Path 1. A deployment follows the documented configuration and sets `required: false`. 2. A protected tool is added or modified without a correct `extra.authInfo` validation step. 3. An unauthenticated attacker connects to the public MCP endpoint. 4. The attacker invokes the affected tool without a bearer token. 5. The wrapper permits the request to reach the MCP handler because authentication is optional. 6. The tool executes its user-data operation because no centralized authorization layer blocks it. The final exploitation step depends on at least one tool lacking the delegated manual check, which is precisely the inse ...[truncated 538 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Use `required: true` for endpoints that can execute protected tools. - Separate unauthenticated discovery operations from authenticated tool execution when the framework permits separate routes or handlers. - If `required: false` is unavoidable, implement a centralized tool-execution guard that denies protected operations unless a validated `AuthInfo` object is present. - Define tools as protected by default and require an explicit declaration for genuinely public tools. - Validate authorization scopes and user ownership in addition to token presence. - Add negative integration tests proving that every protected tool returns an authorization error when invoked without a token, with an invalid token, and with insufficient scopes. - Ensure newly registered tools automatically inherit the guard rather than relying on each developer to add manual checks. ]]>

T09 · Insecure Skill Coding Practices

Warning
Location
SKILL.md:247
Finding
Refresh Token Rotation Leaves Previously Used Tokens Valid<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 247-262 **Vulnerability Type**: Reusable refresh tokens and ineffective token rotation **Risk Level**: Medium ### Complete Code Snippet ```typescript if (body.grant_type === 'refresh_token') { const userId = await redis.get(`refresh:${body.refresh_token}`); if (!userId) return NextResponse.json({ error: 'invalid_grant' }, { status: 400 }); // Optionally refresh upstream tokens here too const newAccess = crypto.randomBytes(16).toString('hex'); const newRefresh = crypto.randomBytes(16).toString('hex'); await redis.set(`mcp_token:${newAccess}`, userId, { ex: 86400 }); await redis.set(`refresh:${newRefresh}`, userId, { ex: 2592000 }); return NextResponse.json({ access_token: newAccess, token_type: 'Bearer', expires_in: 86400, refresh_token: newRefresh, }); } ``` ### Technical Analysis The endpoint issues a new refresh token but never deletes or invalidates the submitted token. This is token duplication rather than secure rotation. Every previously issued refresh token remains usable until its 30-day TTL expires. The implementation also contains no token-family identifier, reuse detection, client binding, or session revocation mechanism. If a refresh token is copied, the legitimate user's later refresh does not terminate the attacker's copy, and repeated use does not trigger revocation. ### Attack Path 1. An attacker obtains a user's refresh token through endpoint compromise, insecure storage, logs, browser data, or another token-disclosure channel. 2. The legitimate client uses that refresh token and receives a replacement token. 3. The original token remains present in Redis because the endpoint does not delete it. 4. The attacker submits the original token to `/api/token`. 5. The server issues a new access token and another refresh token. 6. The attacker repeats this process until the stolen token expires, potentially preserving access by retaining ne ...[truncated 486 chars]
Remediation
<![CDATA[ ## Remediation Suggestions - Perform refresh-token rotation atomically: 1. Validate the submitted token. 2. Mark it as consumed or delete it. 3. Create its replacement. 4. Commit the operation as a single transaction. - Associate refresh tokens with a token-family identifier, client ID, user ID, issuance time, and expiration time. - Detect reuse of a consumed token and revoke the entire token family when reuse occurs. - Revoke related access tokens or session state when compromise is detected. - Bind refresh tokens to the registered OAuth client. - Store only a cryptographic hash of each refresh token where practical, reducing the impact of datastore disclosure. - Provide explicit user and administrator session-revocation mechanisms. ]]>
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
  • Trigger AbuseOverly Broad Trigger, Shadow Command Trigger, Keyword Baiting Trigger
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
Findings (5)

Vague Triggers

Medium
Confidence
94% confidence
Finding
The manifest description says to use the skill whenever the user wants to 'add authentication', 'implement login flow', or even just says 'add auth to my MCP server' or 'my MCP server needs login'. While MCP is a domain constraint, these activation instructions are still broad and lack exclusion conditions or negative examples, which can cause unintended invocation for adjacent authentication tasks.

Intent-Code Divergence

Medium
Confidence
94% confidence
Finding
The authorization flow example is incomplete and under-validates OAuth inputs for a security-sensitive endpoint. It only checks presence of a few parameters and applies weak hostname-based redirect validation, while omitting stronger checks such as exact client/redirect binding, response_type validation, PKCE method enforcement, client registration lookup, and robust state/session handling; implementers may copy this into production and introduce open redirect, authorization code injection, or client impersonation flaws.

Missing User Warnings

Medium
Confidence
92% confidence
Finding
The guide recommends storing upstream access and refresh tokens for long periods in Redis without emphasizing encryption, least retention, access controls, rotation, and user consent. Because these are highly sensitive bearer credentials tied to user accounts, implementers may persist them insecurely and create account-takeover risk if Redis or logs are exposed.

Intent-Code Divergence

Medium
Confidence
90% confidence
Finding
The manifest and guide position the skill as adding OAuth authentication to protect MCP tools. Yet the example sets `required: false` and states that unauthenticated discovery is allowed, meaning the wrapper itself does not enforce authentication on requests and relies on each tool to do so correctly.

Unrestricted Tool Access

Medium
Category
Excessive Agency
Content
},
    {
      "id": 2,
      "prompt": "My MCP server needs user authentication. Users should log in with GitHub OAuth before they can use any tools. The server is on Vercel with Next.js. I need the complete OAuth PKCE flow.",
      "expected_output": "Creates the double OAuth architecture (server for MCP clients + client for GitHub), all well-known endpoints, dynamic client registration, withMcpAuth with required: false, Redis schema"
    }
  ]
Confidence
80% confidence
Finding
Skill grants unrestricted tool access without appropriate constraints. An agent with unfettered tool access can perform arbitrary actions including file modification, network requests, and code execution.

Static analysis

No suspicious patterns detected.