mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-25 08:32:11 +03:00
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.
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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"}]`
|
||||
|
||||
34
open-sse/utils/cookieDomain.ts
Normal file
34
open-sse/utils/cookieDomain.ts
Normal 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();
|
||||
}
|
||||
69
tests/unit/volcengine-cookie-domain-suffix.test.ts
Normal file
69
tests/unit/volcengine-cookie-domain-suffix.test.ts
Normal 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);
|
||||
});
|
||||
Reference in New Issue
Block a user