From 85757a850a5752cda539fb7dd883fd18c700ec5a Mon Sep 17 00:00:00 2001 From: Volnei Munhoz Date: Fri, 20 Feb 2026 07:35:01 -0300 Subject: [PATCH] fix: add CSRF protection to OAuth callback via HMAC-signed nonce (#28083) * fix: add CSRF protection to OAuth callback via HMAC-signed nonce The OAuth state parameter was used only for passing application data (returnTo, teamId) with no cryptographic binding to the user session. An attacker could authorize their own account on a provider, capture the authorization code, and trick a logged-in user into visiting the callback URL to link the attacker's account to the victim's Cal.com profile. Changes: - encodeOAuthState: generate a random nonce and HMAC-sign it with NEXTAUTH_SECRET + userId, injecting both into the OAuth state - decodeOAuthState: verify the HMAC on callback using timingSafeEqual; skip verification when nonce is absent (backwards compatible with apps that don't yet use encodeOAuthState) - Stripe callback: replace raw state.returnTo redirect with getSafeRedirectUrl to prevent open redirect, remove redundant getReturnToValueFromQueryState, add missing return on access_denied Co-Authored-By: Claude Opus 4.6 * fix: make CSRF nonce verification mandatory with allowlist for exempt apps Makes nonce/HMAC verification mandatory by default in decodeOAuthState, preventing attackers from bypassing CSRF protection by omitting nonce fields from the state parameter. Apps not yet migrated to encodeOAuthState (stripe, basecamp3, dub, webex, tandem) are explicitly allowlisted and pass their slug to decodeOAuthState to skip verification. Addresses review feedback (identified by cubic) about the conditional check being trivially bypassable. Co-Authored-By: unknown <> --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../_utils/oauth/decodeOAuthState.ts | 27 +++++++++++++-- .../_utils/oauth/encodeOAuthState.ts | 11 +++++- packages/app-store/basecamp3/api/callback.ts | 2 +- packages/app-store/dub/api/callback.ts | 2 +- .../app-store/stripepayment/api/callback.ts | 34 ++++++------------- .../app-store/tandemvideo/api/callback.ts | 2 +- packages/app-store/types.d.ts | 2 ++ packages/app-store/webex/api/callback.ts | 2 +- 8 files changed, 52 insertions(+), 30 deletions(-) diff --git a/packages/app-store/_utils/oauth/decodeOAuthState.ts b/packages/app-store/_utils/oauth/decodeOAuthState.ts index c300a808c66c36..b8249fc0011e05 100644 --- a/packages/app-store/_utils/oauth/decodeOAuthState.ts +++ b/packages/app-store/_utils/oauth/decodeOAuthState.ts @@ -1,12 +1,35 @@ +import { createHmac, timingSafeEqual } from "node:crypto"; +import process from "node:process"; import type { NextApiRequest } from "next"; - import type { IntegrationOAuthCallbackState } from "../../types"; -export function decodeOAuthState(req: NextApiRequest) { +const NONCE_EXEMPT_APPS = new Set(["stripe", "basecamp3", "dub", "webex", "tandem"]); + +export function decodeOAuthState(req: NextApiRequest, appSlug?: string) { if (typeof req.query.state !== "string") { return undefined; } const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state); + if (appSlug && NONCE_EXEMPT_APPS.has(appSlug)) { + return state; + } + + if (!state.nonce || !state.nonceHash) { + return undefined; + } + + const userId = req.session?.user?.id; + if (!userId || !process.env.NEXTAUTH_SECRET) { + return undefined; + } + const expected = createHmac("sha256", process.env.NEXTAUTH_SECRET) + .update(`${state.nonce}:${userId}`) + .digest(); + const actual = Buffer.from(state.nonceHash, "hex"); + if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) { + return undefined; + } + return state; } diff --git a/packages/app-store/_utils/oauth/encodeOAuthState.ts b/packages/app-store/_utils/oauth/encodeOAuthState.ts index 285642b8c8f207..5721eeb45febc9 100644 --- a/packages/app-store/_utils/oauth/encodeOAuthState.ts +++ b/packages/app-store/_utils/oauth/encodeOAuthState.ts @@ -1,5 +1,6 @@ +import { createHmac, randomUUID } from "node:crypto"; +import process from "node:process"; import type { NextApiRequest } from "next"; - import type { IntegrationOAuthCallbackState } from "../../types"; export function encodeOAuthState(req: NextApiRequest) { @@ -8,5 +9,13 @@ export function encodeOAuthState(req: NextApiRequest) { } const state: IntegrationOAuthCallbackState = JSON.parse(req.query.state); + const userId = req.session?.user?.id; + if (userId && process.env.NEXTAUTH_SECRET) { + state.nonce = randomUUID(); + state.nonceHash = createHmac("sha256", process.env.NEXTAUTH_SECRET) + .update(`${state.nonce}:${userId}`) + .digest("hex"); + } + return JSON.stringify(state); } diff --git a/packages/app-store/basecamp3/api/callback.ts b/packages/app-store/basecamp3/api/callback.ts index 58ea38bf3cdf81..44265b03ffb092 100644 --- a/packages/app-store/basecamp3/api/callback.ts +++ b/packages/app-store/basecamp3/api/callback.ts @@ -87,7 +87,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) }, }); - const state = decodeOAuthState(req); + const state = decodeOAuthState(req, "basecamp3"); res.redirect(getInstalledAppPath({ variant: appConfig.variant, slug: appConfig.slug })); } diff --git a/packages/app-store/dub/api/callback.ts b/packages/app-store/dub/api/callback.ts index 3da4c277f5836b..c4830cb7cb0271 100644 --- a/packages/app-store/dub/api/callback.ts +++ b/packages/app-store/dub/api/callback.ts @@ -13,7 +13,7 @@ import { dubAppKeysSchema } from "../lib/utils"; export default async function handler(req: NextApiRequest, res: NextApiResponse) { const { code } = req.query; - const state = decodeOAuthState(req); + const state = decodeOAuthState(req, "dub"); if (typeof code !== "string") { if (state?.onErrorReturnTo || state?.returnTo) { diff --git a/packages/app-store/stripepayment/api/callback.ts b/packages/app-store/stripepayment/api/callback.ts index b94766ca25eddd..23e4e767246ef8 100644 --- a/packages/app-store/stripepayment/api/callback.ts +++ b/packages/app-store/stripepayment/api/callback.ts @@ -1,36 +1,23 @@ -import type { NextApiRequest, NextApiResponse } from "next"; import { stringify } from "node:querystring"; - +import { getSafeRedirectUrl } from "@calcom/lib/getSafeRedirectUrl"; import type { Prisma } from "@calcom/prisma/client"; - +import type { NextApiRequest, NextApiResponse } from "next"; import getInstalledAppPath from "../../_utils/getInstalledAppPath"; import createOAuthAppCredential from "../../_utils/oauth/createOAuthAppCredential"; import { decodeOAuthState } from "../../_utils/oauth/decodeOAuthState"; import type { StripeData } from "../lib/server"; import stripe from "../lib/server"; -function getReturnToValueFromQueryState(req: NextApiRequest) { - let returnTo = ""; - try { - returnTo = JSON.parse(`${req.query.state}`).returnTo; - } catch (error) { - console.info("No 'returnTo' in req.query.state"); - } - return returnTo; -} - export default async function handler(req: NextApiRequest, res: NextApiResponse) { const { code, error, error_description } = req.query; - const state = decodeOAuthState(req); + const state = decodeOAuthState(req, "stripe"); if (error) { - // User cancels flow if (error === "access_denied") { - state?.onErrorReturnTo ? res.redirect(state.onErrorReturnTo) : res.redirect("/apps/installed/payment"); + return res.redirect(getSafeRedirectUrl(state?.onErrorReturnTo) ?? "/apps/installed/payment"); } const query = stringify({ error, error_description }); - res.redirect(`/apps/installed?${query}`); - return; + return res.redirect(`/apps/installed?${query}`); } if (!req.session?.user?.id) { @@ -43,9 +30,9 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) }); const data: StripeData = { ...response, default_currency: "" }; - if (response["stripe_user_id"]) { - const account = await stripe.accounts.retrieve(response["stripe_user_id"]); - data["default_currency"] = account.default_currency; + if (response.stripe_user_id) { + const account = await stripe.accounts.retrieve(response.stripe_user_id); + data.default_currency = account.default_currency; } await createOAuthAppCredential( @@ -54,6 +41,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) req ); - const returnTo = getReturnToValueFromQueryState(req); - res.redirect(returnTo || getInstalledAppPath({ variant: "payment", slug: "stripe" })); + res.redirect( + getSafeRedirectUrl(state?.returnTo) ?? getInstalledAppPath({ variant: "payment", slug: "stripe" }) + ); } diff --git a/packages/app-store/tandemvideo/api/callback.ts b/packages/app-store/tandemvideo/api/callback.ts index b82ab13b8a755f..128eb1d787847b 100644 --- a/packages/app-store/tandemvideo/api/callback.ts +++ b/packages/app-store/tandemvideo/api/callback.ts @@ -15,7 +15,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) } const code = req.query.code as string; - const state = decodeOAuthState(req); + const state = decodeOAuthState(req, "tandem"); let clientId = ""; let clientSecret = ""; diff --git a/packages/app-store/types.d.ts b/packages/app-store/types.d.ts index 57715fba900964..27050a5ebd8428 100644 --- a/packages/app-store/types.d.ts +++ b/packages/app-store/types.d.ts @@ -12,6 +12,8 @@ export type IntegrationOAuthCallbackState = { installGoogleVideo?: boolean; teamId?: number; defaultInstall?: boolean; + nonce?: string; + nonceHash?: string; }; export type CredentialOwner = { diff --git a/packages/app-store/webex/api/callback.ts b/packages/app-store/webex/api/callback.ts index a43a89784c2c99..8b2840348d6113 100644 --- a/packages/app-store/webex/api/callback.ts +++ b/packages/app-store/webex/api/callback.ts @@ -13,7 +13,7 @@ import { getWebexAppKeys } from "../lib/getWebexAppKeys"; export default async function handler(req: NextApiRequest, res: NextApiResponse) { const { code } = req.query; const { client_id, client_secret } = await getWebexAppKeys(); - const state = decodeOAuthState(req); + const state = decodeOAuthState(req, "webex"); /** @link https://developer.webex.com/docs/integrations#getting-an-access-token **/