Skip to content

fix: prevent OAuth CSRF via missing state parameter in authorization flow#1135

Open
kuranikaran wants to merge 1 commit intogoogle:masterfrom
kuranikaran:fix-oauth-csrf
Open

fix: prevent OAuth CSRF via missing state parameter in authorization flow#1135
kuranikaran wants to merge 1 commit intogoogle:masterfrom
kuranikaran:fix-oauth-csrf

Conversation

@kuranikaran
Copy link

Title

fix: prevent OAuth CSRF via missing state parameter in authorization flow

Body

This PR fixes an OAuth 2.0 CSRF vulnerability in the authorization code flow. Previously, clasp login generated an authorization URL without a state parameter, and the callback handlers accepted any code value without verifying its origin. This allowed an attacker to inject their own authorization code, causing the victim to authenticate as the attacker (account context confusion).

Additionally, .clasprc.json was written with default 0644 permissions, making OAuth tokens readable by any local user or process.

Changes

Added CSRF State Validation:

  • src/auth/auth_code_flow.tsauthorize() now generates a cryptographically random state value (32 bytes, base64url-encoded) and includes it in the authorization URL. parseAuthResponseUrl() now extracts the state parameter from callback URLs.

  • src/auth/localhost_auth_code_flow.ts — The local HTTP callback validates the returned state against the expected value. Mismatched or missing state returns HTTP 400 and rejects the authorization attempt.

  • src/auth/serverless_auth_code_flow.ts — The pasted redirect URL is checked for a matching state parameter before accepting the authorization code.

Hardened Credential Storage:

  • src/auth/file_credential_store.tswriteFile() now writes .clasprc.json with 0600 permissions and calls chmodSync to tighten existing files that may have been created with broader permissions.

Added Tests:

  • test/auth/auth_code_flow.ts — Tests for generateState() uniqueness, URL-safety, and parseAuthResponseUrl() extraction of code, state, and error parameters.

  • test/auth/file_credential_store.ts — Tests that new credential files are created with 0600 permissions and that existing files with 0644 are tightened on write.

Impact

This fix prevents an attacker from injecting their own OAuth authorization code into a victim's clasp login session. Without this fix, a successful attack could result in:

  • Source code theft — victim unknowingly runs clasp push to attacker's project
  • Code injection — victim runs clasp pull and receives attacker-controlled code
  • Persistent access — stolen refresh tokens from world-readable .clasprc.json grant ongoing access to the victim's Apps Script projects and Drive files

Violates OAuth 2.0 security best practices per RFC 6749 Section 10.12 (CSRF Protection). CWE-352 (Cross-Site Request Forgery), CWE-732 (Incorrect Permission Assignment for Critical Resource).

Verification

npm run compile
npx mocha test/auth/auth.ts test/auth/auth_code_flow.ts test/auth/file_credential_store.ts

A mock-based regression checker is included as poc-csrf-demo.cjs:

npm run compile && node poc-csrf-demo.cjs

Against the original source: 4 of 6 checks fail (accept without state validation).
Against the patched source: all 6 checks pass.

@google-cla
Copy link

google-cla bot commented Mar 18, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kuranikaran
Copy link
Author

@sqrrrl this fixes an OAuth CSRF vulnerability in the login flow (missing state parameter per RFC 6749 §10.12) and hardens credential file permissions. Would appreciate a review when you get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants