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. ]]>
