T09 · Insecure Skill Coding Practices
- Location
- scripts/chaoxing-mcp-auto.mjs:256
- Finding
- Missing OAuth State Validation Enables Login CSRF and Account Substitution## Vulnerability Details **File Location**: `scripts/chaoxing-mcp-auto.mjs:256, 294-305, 593-610`; `scripts/chaoxing-mcp-lab.mjs:84, 438-460` **Vulnerability Type**: OAuth login CSRF / authorization response injection **Risk Level**: Medium ### Technical Analysis Both OAuth implementations generate predictable `state` values derived from the requested scope rather than cryptographically random, transaction-specific values: ```javascript // scripts/chaoxing-mcp-auto.mjs:250-258 function buildAuthUrl(cred, scope) { const u = new URL(AUTHORIZE_URL); u.searchParams.set('client_id', cred.clientId); u.searchParams.set('response_type', 'code'); u.searchParams.set('redirect_uri', REDIRECT_URI); if (scope !== null && scope !== undefined) u.searchParams.set('scope', scope); u.searchParams.set('state', 'auto-' + (scope ?? '')); return u.toString(); } ``` The callback handler accepts any supplied authorization code without retrieving or validating the returned `state`: ```javascript // scripts/chaoxing-mcp-auto.mjs:294-305 if (u.pathname === '/callback') { const code = u.searchParams.get('code'); const err = u.searchParams.get('error'); const desc = u.searchParams.get('error_description'); if (!code) { log(`授权失败:${err} / ${desc}`); res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); return res.end(page(`<h1 class="fail">✘ 授权未通过:${esc(err || '')} ${esc(desc || '')}</h1><p><a href="/lab">← 返回重试(检查 scope 取值)</a></p>`)); } log('收到授权码,正在换取令牌…'); const { status, json } = await exchangeCode(cred, code); ``` The setup callback has the same weakness and applies the resulting tokens to local state and WorkBuddy configuration: ```javascript // scripts/chaoxing-mcp-auto.mjs:593-610 if (u.pathname === '/callback') { const code = u.searchParams.get('code'); const err = u.searchParams.get('error'); const desc = u.searchParams.get('error_description'); const cred = pendingCred || (await getCreds()); if (!cred) { r ...[truncated 3805 chars]
- Remediation
- ## Remediation Suggestions 1. Generate a new state value for each authorization attempt using a cryptographically secure source such as `crypto.randomBytes(32).toString('base64url')`. 2. Store the expected state only in process memory together with its creation time and intended authorization parameters. 3. Require an exact state match before exchanging an authorization code. 4. Reject callbacks with missing, unknown, expired, or previously consumed state values. 5. Consume the state before or atomically with code exchange to prevent callback replay. 6. Do not encode scope or other predictable values as the security state. Store such metadata alongside the random state in local process memory. 7. Close the callback server after the first valid authorization transaction. 8. Use Authorization Code with PKCE if the provider supports it, while retaining state validation for CSRF protection. 9. Apply the same correction to `runAuthLab`, `runSetup`, and `chaoxing-mcp-lab.mjs` so no alternative entry point remains vulnerable.
