Compare commits

...

1 Commits

Author SHA1 Message Date
Xiangzhe
aa597ab0c1 fix(security): match cookie domains by suffix, not substring
CodeQL js/incomplete-url-substring-sanitization, alerts #860 and #861:
volcengineConsoleAutoLogin accepted any cookie whose `domain` merely *contained*
"volcengine.com".

That check is an authorization decision, not a string test. The console
auto-login harvests `digest`, `AccountID`, `csrfToken` and `userInfo` out of the
Playwright context and persists them as the operator's Volcengine credentials,
so a cookie set by `volcengine.com.attacker.tld` — or `notvolcengine.com` — was
captured and stored as a provider connection.

Add `matchesCookieDomain()` (open-sse/utils/cookieDomain.ts): exact host or
dot-boundary suffix, leading dots and case normalized on both sides, failing
closed on an empty expected domain. Same shape as the existing
`isAdobeCookieDomain` in adobeFireflyBrowserLogin.ts, which already got this
right.

While sweeping the class, inAppLoginService's cookie capture had the identical
weakness — `c.domain.includes(domain.replace(/^\./, ""))` — with the identical
consequence: a look-alike host's cookie stored as the operator's credential.
CodeQL did not flag it because the expected domain comes from
TOKEN_EXTRACTION_CONFIGS rather than a literal. Both callsites now share the
helper.

tests/unit/volcengine-cookie-domain-suffix.test.ts — 5 tests, red before the
fix, covering the real domains, seven look-alikes, empty/missing input, and the
config-supplied path.
2026-08-24 15:57:16 -03:00
4 changed files with 128 additions and 3 deletions

View File

@@ -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 `<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;

View File

@@ -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<void> {
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"}]`

View File

@@ -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();
}

View File

@@ -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);
});