mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-14 14:23:14 -06:00
d26aaebef6
Five reviewers went at the two SSO commits. Three of them independently
demonstrated a full account takeover, and it was the same defect each time.
TAKEOVER. An org admin supplies the issuer and client_id, so they control that
identity provider completely and can mint an id_token asserting ANY email with
email_verified:true — including a platform_admin's. Every cryptographic check
passed honestly, because the attacker IS the issuer. upsertFederatedUser then
re-pointed the existing account at whichever provider spoke last, because the
only guard was `password_hash IS NULL` — and every SSO-created account has a
null password. Sessions were issued as the victim, and the victim's own login
then failed forever with subject_mismatch.
The rule came from the old Google handler, where it was safe: only the operator
could add a provider. Making providers customer-configurable turned it into a
takeover primitive and the assumption was not re-examined. Now an org provider
may only assert emails inside the domains it registered, and may never adopt an
account another provider established.
REMOTE CRASH, unauthenticated. The state comparison guarded on UTF-16 character
length while Buffer.from produces UTF-8 bytes, so a state of 43 characters
containing one multi-byte character reached timingSafeEqual with mismatched
buffers and threw — inside an async handler, which Express does not catch, which
server.js turns into process.exit. One request per restart killed any instance
with SSO enabled. Compared as bytes now, and /api/auth/oidc gained a rate limit.
LOGIN CSRF. The callback returned the session token in the URL fragment, so a
crafted link installed an ATTACKER'S token and silently signed the victim into
their account. The token now goes in a one-shot httpOnly cookie exchanged at
POST /sso/claim, which a link cannot forge.
FRONTEND, dead on arrival twice over. login.js used `await` in a non-async
function — a SyntaxError that takes the WHOLE app down, since app.js imports it
statically and there is no bundler. And `esc` was never imported, so the org-SSO
button could never render; the ReferenceError was swallowed by the catch written
for network failures. Both slipped through because `node --check` parses these
files as CommonJS and exits 0 on a broken module. The correct check is
`node --input-type=module --check`, and all four frontend files now pass it.
PUBLIC EMAIL DOMAINS cannot be claimed. A tenant had claimed gmail.com in
review, after which every Gmail user typing their address was offered "sign in
with your organization" pointing at that tenant's infrastructure — phishing from
this product's own login page. server/lib/public-email-domains.js.
MICROSOFT multi-tenant is refused rather than silently broken. `common` metadata
advertises the literal template {tenantid}, so the issuer never matches and
every login already failed; and loosening that check is nOAuth. A tenant GUID is
now required, with a loud warning at boot.
SSRF: https only, loopback/RFC1918/link-local refused, redirects not followed,
and the test endpoint no longer echoes upstream status for a caller-supplied
jwks_uri (it was a readable internal port scanner).
Also: an omitted email_verified was accepted (the comment already said it should
not be); the domain-uniqueness check raced an 8s network call before its insert
and is now inside the transaction; same-org duplicate domains were allowed and
made routing depend on table-scan order; routing is now ordered; a client secret
that cannot be decrypted fails closed instead of silently downgrading to a public
client; SSO audit rows were writing the org id into the deviceId column; and
/sso/start was capped at 10/min per IP, which would 429 the 11th employee behind
a corporate NAT.
Adds per-provider editing in the org admin UI (replace-only secrets — never
returned, blank means keep, explicit clear) and a Test button that checks
discovery, endpoints and signing keys while stating plainly that it cannot
verify the client ID, the secret, or the redirect URI registration.
⚠️ STILL MISSING: domain-ownership verification. A claimed domain means "nobody
else had claimed it", not "they own it". DNS TXT proof is the remaining control.
1582 tests pass. New regression tests cover the takeover confinement, ordering,
fail-closed secrets, the Microsoft refusal and the public-domain blocklist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
58 lines
3.2 KiB
JavaScript
58 lines
3.2 KiB
JavaScript
'use strict';
|
|
|
|
/*
|
|
* Email domains nobody may claim for organization SSO.
|
|
*
|
|
* Per-org SSO routes everyone at a domain to that organization's identity provider. Applied to a
|
|
* company domain that is the point. Applied to a CONSUMER domain it is an attack: one tenant claims
|
|
* `gmail.com`, and from then on every Gmail user who types their address into this product's login
|
|
* page is offered a "sign in with your organization" button that sends them to infrastructure the
|
|
* tenant controls — phishing launched from the vendor's own trusted login screen. First-claim-wins
|
|
* also lets one cheap account deny a public domain to everyone else, permanently.
|
|
*
|
|
* ⚠️ This is a floor, not a ceiling. It stops the mass-abuse case; it does NOT stop a tenant
|
|
* claiming a domain that belongs to some specific other company. Only proof of control — a DNS TXT
|
|
* record, or a challenge to postmaster@ — settles that, and until it exists a claimed domain means
|
|
* "nobody else had claimed it", not "they own it".
|
|
*
|
|
* Kept as data, in one file, because it is a list that will need adding to and that is the cheapest
|
|
* possible edit. Matching is exact on the registrable domain, so `mail.google.com` is not blocked by
|
|
* `gmail.com` — subdomains of consumer providers are not a realistic sign-in domain anyway.
|
|
*/
|
|
|
|
const PUBLIC_EMAIL_DOMAINS = new Set([
|
|
// Google
|
|
'gmail.com', 'googlemail.com',
|
|
// Microsoft
|
|
'outlook.com', 'outlook.co.uk', 'hotmail.com', 'hotmail.co.uk', 'hotmail.fr', 'hotmail.it',
|
|
'live.com', 'live.co.uk', 'msn.com', 'passport.com',
|
|
// Yahoo and friends
|
|
'yahoo.com', 'yahoo.co.uk', 'yahoo.co.jp', 'yahoo.fr', 'yahoo.de', 'yahoo.ca', 'yahoo.com.au',
|
|
'ymail.com', 'rocketmail.com', 'aol.com', 'aim.com',
|
|
// Apple
|
|
'icloud.com', 'me.com', 'mac.com',
|
|
// Privacy-focused
|
|
'proton.me', 'protonmail.com', 'pm.me', 'tutanota.com', 'tutanota.de', 'tuta.io', 'tuta.com',
|
|
'duck.com', 'hey.com', 'fastmail.com', 'fastmail.fm',
|
|
// Other large consumer providers
|
|
'gmx.com', 'gmx.de', 'gmx.net', 'gmx.at', 'gmx.ch', 'web.de', 'mail.com', 'email.com',
|
|
'zoho.com', 'zohomail.com', 'yandex.com', 'yandex.ru', 'ya.ru', 'mail.ru', 'bk.ru', 'inbox.ru',
|
|
'list.ru', 'rambler.ru',
|
|
'qq.com', 'foxmail.com', '163.com', '126.com', 'sina.com', 'sina.cn', 'naver.com', 'daum.net',
|
|
'hanmail.net', 'rediffmail.com',
|
|
// ISP-style mailboxes, where the domain belongs to the ISP and not to any customer
|
|
'comcast.net', 'verizon.net', 'att.net', 'sbcglobal.net', 'bellsouth.net', 'cox.net',
|
|
'charter.net', 'earthlink.net', 'juno.com', 'optonline.net', 'roadrunner.com',
|
|
'btinternet.com', 'sky.com', 'virginmedia.com', 'talktalk.net', 'orange.fr', 'wanadoo.fr',
|
|
'free.fr', 'laposte.net', 'libero.it', 'virgilio.it', 'tiscali.it', 'terra.com.br', 'uol.com.br',
|
|
'bol.com.br', 'telus.net', 'shaw.ca', 'rogers.com', 'sympatico.ca', 'bigpond.com', 'optusnet.com.au',
|
|
't-online.de', 'freenet.de', 'arcor.de',
|
|
]);
|
|
|
|
/** True when this domain is a consumer mailbox provider rather than an organization's own domain. */
|
|
function isPublicEmailDomain(domain) {
|
|
return PUBLIC_EMAIL_DOMAINS.has(String(domain || '').trim().toLowerCase());
|
|
}
|
|
|
|
module.exports = { PUBLIC_EMAIL_DOMAINS, isPublicEmailDomain };
|