diff --git a/open-sse/services/inAppLoginService.ts b/open-sse/services/inAppLoginService.ts index d2196fd6d0..292c15ad5b 100644 --- a/open-sse/services/inAppLoginService.ts +++ b/open-sse/services/inAppLoginService.ts @@ -18,6 +18,7 @@ import { TokenExtractionConfig, type TokenSource, } from "./tokenExtractionConfig"; +import { matchesCookieDomain } from "../utils/cookieDomain"; // ─── Types ────────────────────────────────────────────────────────────────── @@ -196,9 +197,14 @@ export class InAppLoginService extends EventEmitter { for (const source of tokenSources) { if (source.type === "cookie") { const domain = source.domain || undefined; + // Exact host or dot-boundary suffix, never `includes()`: a cookie + // from `.attacker.tld` would otherwise be captured and + // persisted as the operator's credential. Same class CodeQL flagged + // in volcengineConsoleAutoLogin (#860/#861); this callsite was not + // flagged because the expected domain is config-supplied. const matched = cookies.find( (c: any) => - c.name === source.name && (!domain || c.domain.includes(domain.replace(/^\./, ""))) + c.name === source.name && (!domain || matchesCookieDomain(c.domain, domain)) ); if (matched && !credentials[source.name]) { credentials[source.name] = matched.value; diff --git a/open-sse/services/volcengineConsoleAutoLogin.ts b/open-sse/services/volcengineConsoleAutoLogin.ts index 6048cbb63f..0e3b681820 100644 --- a/open-sse/services/volcengineConsoleAutoLogin.ts +++ b/open-sse/services/volcengineConsoleAutoLogin.ts @@ -28,6 +28,7 @@ */ import { randomUUID } from "crypto"; +import { matchesCookieDomain } from "../utils/cookieDomain"; // ─── Public types ─────────────────────────────────────────────────────────── @@ -231,6 +232,21 @@ export function normalizePhone(raw: string): string | null { return /^1\d{10}$/.test(bare) ? bare : null; } +/** + * Whether a cookie's `domain` belongs to the Volcengine console. + * + * Cookie domains must be matched by exact host or dot-boundary suffix, never by + * substring: `domain.includes("volcengine.com")` also accepted + * `volcengine.com.attacker.tld` and `notvolcengine.com`, so a cookie named + * `digest`/`AccountID`/`csrfToken`/`userInfo` set by a look-alike host was + * harvested as an operator credential and persisted as a provider connection + * (CodeQL js/incomplete-url-substring-sanitization #860/#861). Mirrors + * `isAdobeCookieDomain` in adobeFireflyBrowserLogin.ts. + */ +export function isVolcengineCookieDomain(domain: string | undefined): boolean { + return matchesCookieDomain(domain, "volcengine.com"); +} + function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -618,7 +634,7 @@ export class VolcengineConsoleAutoLoginService { for (const cookie of cookies as Array<{ name: string; domain: string; value: string }>) { if ( REQUIRED_COOKIES.includes(cookie.name as (typeof REQUIRED_COOKIES)[number]) && - cookie.domain.includes("volcengine.com") + isVolcengineCookieDomain(cookie.domain) ) { credentials[cookie.name] = cookie.value; } @@ -763,7 +779,7 @@ export class VolcengineConsoleAutoLoginService { domain: string; }>; const present = REQUIRED_COOKIES.filter((name) => - cookies.some((c) => c.name === name && c.domain.includes("volcengine.com")) + cookies.some((c) => c.name === name && isVolcengineCookieDomain(c.domain)) ); parts.push( `cookies=[${present.join(",") || "none of digest/AccountID/csrfToken/userInfo"}]` diff --git a/open-sse/utils/cookieDomain.ts b/open-sse/utils/cookieDomain.ts new file mode 100644 index 0000000000..af704b26a4 --- /dev/null +++ b/open-sse/utils/cookieDomain.ts @@ -0,0 +1,34 @@ +/** + * Cookie-domain matching for browser-driven credential capture. + * + * Every in-app / console login flow harvests cookies out of a Playwright + * context and persists them as operator credentials, so "is this cookie from + * the site I sent the browser to?" is an authorization decision. A substring + * test is not one: `domain.includes("example.com")` also accepts + * `example.com.attacker.tld` and `notexample.com`, which lets a look-alike host + * hand us cookies we then store as the operator's real credentials + * (CodeQL js/incomplete-url-substring-sanitization). + * + * A cookie domain is matched by exact host or dot-boundary suffix — nothing + * else. Leading dots (the RFC 6265 "domain-matches any subdomain" spelling) and + * case are normalized away on both sides. + */ +export function matchesCookieDomain( + cookieDomain: string | undefined, + expectedDomain: string | undefined +): boolean { + const expected = normalizeCookieDomain(expectedDomain); + if (!expected) return false; + + const actual = normalizeCookieDomain(cookieDomain); + if (!actual) return false; + + return actual === expected || actual.endsWith(`.${expected}`); +} + +function normalizeCookieDomain(domain: string | undefined): string { + return String(domain || "") + .trim() + .replace(/^\.+/, "") + .toLowerCase(); +} diff --git a/tests/unit/volcengine-cookie-domain-suffix.test.ts b/tests/unit/volcengine-cookie-domain-suffix.test.ts new file mode 100644 index 0000000000..bb55ee1fca --- /dev/null +++ b/tests/unit/volcengine-cookie-domain-suffix.test.ts @@ -0,0 +1,69 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { isVolcengineCookieDomain } from "../../open-sse/services/volcengineConsoleAutoLogin.ts"; + +// CodeQL js/incomplete-url-substring-sanitization (#860, #861). The console +// auto-login harvested `digest`/`AccountID`/`csrfToken`/`userInfo` from any +// cookie whose domain merely *contained* "volcengine.com", so a cookie set by +// `volcengine.com.attacker.tld` (or `notvolcengine.com`) was accepted as an +// operator credential and persisted as a provider connection. Match the domain +// the way a cookie domain has to be matched: exact host or a dot-boundary +// suffix. Mirrors isAdobeCookieDomain in adobeFireflyBrowserLogin.ts. + +test("accepts the real console cookie domains", () => { + for (const domain of [ + "volcengine.com", + ".volcengine.com", + "console.volcengine.com", + ".console.volcengine.com", + "CONSOLE.VOLCENGINE.COM", + " .volcengine.com ", + ]) { + assert.equal(isVolcengineCookieDomain(domain), true, domain); + } +}); + +test("rejects look-alike domains that merely contain the string", () => { + for (const domain of [ + "volcengine.com.attacker.tld", + ".volcengine.com.evil.example", + "notvolcengine.com", + "myvolcengine.com", + "volcengine.com.br", + "evil.tld/volcengine.com", + "volcengine.company", + ]) { + assert.equal(isVolcengineCookieDomain(domain), false, domain); + } +}); + +test("rejects empty / missing domains instead of throwing", () => { + assert.equal(isVolcengineCookieDomain(undefined), false); + assert.equal(isVolcengineCookieDomain(""), false); + assert.equal(isVolcengineCookieDomain(" "), false); +}); + +// The same class exists in inAppLoginService's cookie capture, where the +// expected domain comes from TOKEN_EXTRACTION_CONFIGS instead of a literal — +// which is why CodeQL did not flag it. Same helper, same guarantees. + +test("matchesCookieDomain handles a config-supplied expected domain", async () => { + const { matchesCookieDomain } = await import("../../open-sse/utils/cookieDomain.ts"); + + assert.equal(matchesCookieDomain("app.example.com", "example.com"), true); + assert.equal(matchesCookieDomain(".example.com", ".example.com"), true); + assert.equal(matchesCookieDomain("example.com", ".example.com"), true); + + assert.equal(matchesCookieDomain("example.com.attacker.tld", "example.com"), false); + assert.equal(matchesCookieDomain("notexample.com", "example.com"), false); + assert.equal(matchesCookieDomain("example.com", "app.example.com"), false); +}); + +test("matchesCookieDomain fails closed on a missing expected domain", async () => { + const { matchesCookieDomain } = await import("../../open-sse/utils/cookieDomain.ts"); + + assert.equal(matchesCookieDomain("example.com", undefined), false); + assert.equal(matchesCookieDomain("example.com", ""), false); + assert.equal(matchesCookieDomain("example.com", "."), false); +});