Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion open-sse/services/inAppLoginService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
TokenExtractionConfig,
type TokenSource,
} from "./tokenExtractionConfig";
import { matchesCookieDomain } from "../utils/cookieDomain";

// ─── Types ──────────────────────────────────────────────────────────────────

Expand Down Expand Up @@ -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 `<domain>.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;
Expand Down
20 changes: 18 additions & 2 deletions open-sse/services/volcengineConsoleAutoLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*/

import { randomUUID } from "crypto";
import { matchesCookieDomain } from "../utils/cookieDomain";

// ─── Public types ───────────────────────────────────────────────────────────

Expand Down Expand Up @@ -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<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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"}]`
Expand Down
34 changes: 34 additions & 0 deletions open-sse/utils/cookieDomain.ts
Original file line number Diff line number Diff line change
@@ -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();
}
69 changes: 69 additions & 0 deletions tests/unit/volcengine-cookie-domain-suffix.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
Loading