T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/ticktick-cli.mjs:152
- Finding
- OAuth State Is Predictable and Not Validated During Authorization-Code Exchange<![CDATA[ ## Vulnerability Details **File Location**: `skill-entry/token-manager.mjs:28-30, 230-236`; `scripts/ticktick-cli.mjs:152-181` **Vulnerability Type**: OAuth login CSRF and authorization-code substitution **Risk Level**: Medium ### Vulnerable Code `skill-entry/token-manager.mjs:28-30`: ```js export function createOAuthState(prefix = "oc") { return `${prefix}_${Date.now().toString(36)}`; } ``` `skill-entry/token-manager.mjs:230-236`: ```js export function parseCallbackUrl(callbackUrl) { const parsed = new URL(callbackUrl); const code = parsed.searchParams.get("code") ?? undefined; const state = parsed.searchParams.get("state") ?? undefined; const error = parsed.searchParams.get("error") ?? undefined; const errorDescription = parsed.searchParams.get("error_description") ?? undefined; return { code, state, error, errorDescription }; } ``` `scripts/ticktick-cli.mjs:152-181`: ```js if (parsed.command === "auth-url") { const state = readFlag(parsed, "state") ?? createOAuthState(); const authUrl = buildTickTickAuthUrl(env, state); console.log(JSON.stringify({ state, authUrl, redirectUri: env.redirectUri }, null, 2)); return; } if (parsed.command === "auth-exchange") { const callbackUrl = readFlag(parsed, "callbackUrl"); const codeFromFlag = readFlag(parsed, "code"); let code = codeFromFlag; let state; if (callbackUrl) { const parsedCallback = parseCallbackUrl(callbackUrl); if (parsedCallback.error) { throw new Error( `OAuth callback returned error='${parsedCallback.error}'${ parsedCallback.errorDescription ? ` description='${parsedCallback.errorDescription}'` : "" }` ); } code = parsedCallback.code; state = parsedCallback.state; } if (!code) { throw new Error("auth-exchange requires --callbackUrl <url> or --code <code>"); } const token = await exchangeCodeAndPersistToken({ code, tokenPath, env }); ``` ### Technical Analysis The OAuth `state` p ...[truncated 2212 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Generate state with a cryptographically secure random source: ```js import { randomBytes } from "node:crypto"; export function createOAuthState() { return randomBytes(32).toString("base64url"); } ``` 2. Persist the issued state in a file created with mode `0600`, together with its creation time and intended redirect URI. 3. Require the callback to contain a state value and reject missing, expired, malformed, or mismatched values. 4. Compare state values using `crypto.timingSafeEqual` after validating equal lengths. 5. Delete or invalidate stored state immediately after one successful exchange to prevent replay. 6. Disable bare `--code` exchange by default. If it is necessary for advanced workflows, require an explicit unsafe/manual-flow option and a separately supplied expected state. 7. Add PKCE using a securely generated verifier and `S256` challenge, persist the verifier with state, and include it during token exchange. 8. Add unit tests covering missing state, mismatched state, replayed state, expired state, and successful one-time validation. ]]>