Engagement Report: @slack/oauth's Legacy State Mode Breaks Its Own CSRF Protection

Another in the series of redacted-format engagement reports, this one on @slack/oauth, Slack's own official package for building the "Add to Slack" OAuth install flow. Any app that lets a user connect their Slack workspace, from a one-person side project to an enterprise integration, is a plausible consumer of this exact code. That makes the target here specifically the state parameter verification path, the mechanism that's supposed to guarantee the browser completing an OAuth callback is the same one that started it.

Scope and Objective

Target: InstallProvider.handleCallback() and ClearStateStore.verifyStateParam() in packages/oauth/src/install-provider.ts and packages/oauth/src/state-stores/clear-state-store.ts, cloned directly from slackapi/node-slack-sdk.

Objective: read the real CSRF-protection logic end to end, not the README's description of it, and check whether every configuration the package actually ships still enforces session binding on the state parameter. Verification meant extracting the real ClearStateStore class verbatim and running it, unmodified, against a crafted scenario in a disposable directory, never against a live Slack workspace or any third party's deployment.

Findings

legacyStateVerification drops session binding entirely (High)

InstallProvider's default behavior, the code that ships when a developer doesn't touch this option, is correct. handleCallback() reads a state value out of an HttpOnly cookie set on the browser that started the flow, compares it against the state query parameter on the callback, and rejects the request if they don't match:

const stateInBrowserSession: string | undefined = extractCookieValue(req, this.stateCookieName);
if (!stateInBrowserSession || stateInBrowserSession !== stateInQueryString) {
  throw new InvalidStateError('The state parameter is not for this browser session.');
}

That's the actual CSRF defense: binding the value to a specific browser's cookie jar means an attacker can't hand a victim a link carrying a state the attacker generated for themselves. But the package also ships a constructor flag, legacyStateVerification, "intended... for some of the existing developers that need time for migration." When it's true, the cookie comparison above is skipped entirely:

if (this.legacyStateVerification) {
  // This mode is not enabled by default
  this.logger.warn(
    'Enabling legacyStateVerification is not recommended as it does not properly work for OAuth CSRF protection...',
  );
} else {
  const stateInBrowserSession = extractCookieValue(req, this.stateCookieName);
  if (!stateInBrowserSession || stateInBrowserSession !== stateInQueryString) {
    throw new InvalidStateError('The state parameter is not for this browser session.');
  }
}
if (this.stateStore) {
  installOptions = await this.stateStore.verifyStateParam(new Date(), stateInQueryString);
}

With legacyStateVerification set, the only remaining check is stateStore.verifyStateParam(), and the package's own default StateStore implementation, ClearStateStore, verifies exactly two things about a state value: that it's a correctly-signed JWT, and that it isn't older than the expiration window (600 seconds by default). Neither check has any concept of which browser the value belongs to. A state the package itself generated for an attacker's own install attempt passes with the same validity as one generated for the victim.

The maintainers clearly know this. The warning string says so directly: "does not properly work for OAuth CSRF protection." What's notable isn't that the weakness exists, it's that it ships, live, in the current package, gated behind nothing but a logger.warn() call that a production deployment can easily never see.

Verification

Rather than trust the code's description of itself, I extracted ClearStateStore verbatim from clear-state-store.ts into a standalone script (same jsonwebtoken dependency the real class imports, same method bodies, no logic changed) and ran it in a disposable directory:

$ node verify_state_store.js

=== Step 1: ATTACKER starts their own OAuth install flow ===
Attacker's own browser requests /slack/install, server issues state:
  state = eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ...

=== Step 3: VICTIM's browser hits the callback (legacyStateVerification=true path) ===
Calling the REAL verifyStateParam() with the attacker-issued state, as the victim:
RESULT: ACCEPTED. No error thrown.
  decoded installOptions: {"scopes":["chat:write"],"teamId":"ATTACKER_OWN_TEST_WORKSPACE"}

The state the attacker generated for their own flow was accepted as valid for a request coming from a completely different browser, with no cookie involved in the check at all. That's the exploitable condition, confirmed against the real function, not a paraphrase of it.

Why this isn't just a flaw, it's a chain

A verified gap in session binding isn't a complete attack by itself; it's the precondition for one, and the shape it takes is already a documented class, not a novel idea. PortSwigger's Web Security Academy runs a lab called "Forced OAuth profile linking" built on exactly this precondition: a linking endpoint with no state parameter protecting it. The lab's chain is the template here too. An attacker starts their own OAuth flow against the target application, intercepts the request before it completes, and sends a victim, someone already holding an authenticated session on the target app, a crafted link or auto-submitting page carrying the attacker's own state and code. The victim's browser, already logged in, completes the flow on the attacker's behalf. Whatever the application does with a successful callback, link this Slack workspace to the current account, install this integration under the current user, happens using the attacker's OAuth grant but the victim's session identity.

Applied to @slack/oauth: the SDK's weak state check is the lock that's missing. The actual damage depends on what the consuming application does in its afterInstallation handler or installationStore.storeInstallation() call, which is app-specific and outside this package. For an app using Slack OAuth as "connect your workspace to your existing account," the PortSwigger pattern maps over directly: forced linking of an attacker-controlled Slack identity to a victim's authenticated session, the precursor to account takeover. For an app using it purely to add a bot to a workspace, the consequence shifts toward installing attacker-chosen scopes into a victim-initiated flow. Either way, the state parameter was supposed to be the one thing standing between "this request is who it says it is" and "this request came from wherever it came from." legacyStateVerification ships with that guarantee quietly removed.

Mitigation

Don't set legacyStateVerification: true. If an existing deployment has it set for a migration that never finished, that migration is the actual security debt, not a configuration nice-to-have. The package's own warning line is accurate; take it as a hard blocker, not a lint note.