mirror of
https://github.com/PR0M3TH3AN/bitgate.git
synced 2026-10-31 10:31:00 +00:00
fix(security): close every finding from the audit
Critical - Any pubkey could publish a kind:30078 roles document and become root of a deployment, granting itself every capability and denying anyone. Roles are now accepted only from the configured root, compared against the constructor value rather than current authority state so a bad ingest cannot move the goalposts. Reproduced end to end before the fix; regression test added. - Administrative state now fails closed without signature verification. Without it, event.pubkey is an unauthenticated claim, so an authorship check alone proves nothing — a relay can put the root's key on an event the root never wrote. trustUnsignedEvents opts in for local and test use. High - Report categories could evade moderation entirely: resolveThresholds used bracket truthiness, so "constructor" resolved to the Object constructor (all gates undefined) and "__proto__" to Object.prototype, both bypassing the default threshold. Own-property lookups and null-prototype tables throughout core — policy, evaluator, evidence, authority, presets. - Live subscriptions bypassed signature verification entirely; an attacker only had to wait for the subscription rather than the load. - The default administrative query asked relays for every kind:30078 and kind:30000 event in existence, with no author, identifier, or limit. Now filtered to governance identifiers with a bounded limit. Medium - Cached state was trusted as authority: a cache naming a different root is now rejected, so same-origin tampering cannot persist a takeover. - Provider attribute changes silently stopped working after the first one, and concurrent starts leaked a transport. Generation-guarded. - Unbounded growth in the report store, mute store, decision cache, and the transport's per-subscription dedupe set — all now capped. Also restores the on-wire `d` identifier to <namespace>:governance:<scope>:v1. The BitGate rename replaced "governance:" too broadly and silently changed the event format; both encoder and parser changed together, so round-trip tests never noticed. The identifier names the kind of document, not the library. Adds docs/security.md and 21 regression tests reproducing each finding. 832 tests.
This commit is contained in:
1 parent
7a45f1cf3e
commit
94ef6a72f5
21 files changed
+788
-54
No files matched your search
@@ -281,7 +281,7 @@ export class GovernanceCommands {
|
||||
kind: 30078,
|
||||
content: JSON.stringify(policy),
|
||||
tags: [
|
||||
["d", `${this.runtime.namespace}:bitgate:policy:v1`],
|
||||
["d", `${this.runtime.namespace}:governance:policy:v1`],
|
||||
["v", "1"],
|
||||
["client", "bitgate"],
|
||||
["scope", "policy"],
|
||||
@@ -307,7 +307,7 @@ export class GovernanceCommands {
|
||||
}
|
||||
|
||||
const tags = [
|
||||
["d", `${this.runtime.namespace}:bitgate:community-sources:v1`],
|
||||
["d", `${this.runtime.namespace}:governance:community-sources:v1`],
|
||||
["v", "1"],
|
||||
["client", "bitgate"],
|
||||
["scope", "community-sources"],
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
/**
|
||||
* @typedef {Object} ListOptions
|
||||
* @property {number} [timeout]
|
||||
* @property {number} [maxSeen] - Ceiling on the per-subscription dedupe window
|
||||
* @property {string[]} [relays] - Query only these relays, connecting if needed.
|
||||
* This is what makes outbox-model fetching possible: a contact's mute list
|
||||
* lives on their write relays, not on this deployment's.
|
||||
|
||||
@@ -316,6 +316,11 @@ export function createRelayTransport(urls, options = {}) {
|
||||
subscribe(filters, handlers, subscribeOptions = {}) {
|
||||
const id = `bg-sub-${(subscriptionCounter += 1)}`;
|
||||
const targets = resolveTargets(subscribeOptions.relays);
|
||||
// Bounded: a long-lived subscription on a busy relay would otherwise
|
||||
// retain every event id it has ever seen for the life of the page. The
|
||||
// window only needs to be wide enough to catch the same event arriving
|
||||
// from several relays at once.
|
||||
const maxSeen = subscribeOptions.maxSeen ?? 10_000;
|
||||
/** @type {Set<string>} */
|
||||
const seen = new Set();
|
||||
|
||||
@@ -327,6 +332,12 @@ export function createRelayTransport(urls, options = {}) {
|
||||
if (!event?.id || seen.has(event.id)) {
|
||||
return;
|
||||
}
|
||||
if (seen.size >= maxSeen) {
|
||||
const oldest = seen.values().next().value;
|
||||
if (oldest !== undefined) {
|
||||
seen.delete(oldest);
|
||||
}
|
||||
}
|
||||
seen.add(event.id);
|
||||
handlers.onEvent?.(event);
|
||||
},
|
||||
|
||||
+188
-14
@@ -39,6 +39,30 @@ import {
|
||||
import { snapshotFingerprint } from "@bitgate/core";
|
||||
|
||||
import { Emitter } from "./emitter.js";
|
||||
|
||||
/** Governance document scopes, used to build a bounded `#d` filter. */
|
||||
const GOVERNANCE_SCOPES = [
|
||||
"roles",
|
||||
"policy",
|
||||
"user-allow",
|
||||
"user-deny",
|
||||
"event-deny",
|
||||
"address-deny",
|
||||
"trust-seed",
|
||||
"community-sources",
|
||||
];
|
||||
|
||||
/** Legacy administrative list identifiers, read for migration compatibility. */
|
||||
const LEGACY_IDENTIFIERS = [
|
||||
"bitvid:admin:whitelist",
|
||||
"bitvid:admin:blacklist",
|
||||
"bitvid:admin:event-blacklist",
|
||||
"bitvid:admin:editors",
|
||||
"bitvid:admin:community-sources",
|
||||
];
|
||||
|
||||
/** Maximum administrative events accepted from one query. */
|
||||
const DEFAULT_ADMIN_LIMIT = 500;
|
||||
import { createMemoryStorage, createNullSigner, storageKey } from "./interfaces.js";
|
||||
import {
|
||||
GovernanceAdminStore,
|
||||
@@ -116,7 +140,12 @@ export class GovernanceRuntime extends Emitter {
|
||||
* @param {number} [options.chunkSize]
|
||||
* @param {number} [options.muteWindowSeconds]
|
||||
* @param {import('@bitgate/nostr').SignatureVerifier} [options.verifySignature]
|
||||
* @param {boolean} [options.trustUnsignedEvents] - Accept administrative events
|
||||
* without a verifier. Development and tests only.
|
||||
* @param {string} [options.root] - Root administrator pubkey, from deployment config
|
||||
* @param {number} [options.maxCachedDecisions] - Ceiling on cached decisions
|
||||
* @param {number} [options.maxReportTargets] - Ceiling on targets held by the report store
|
||||
* @param {number} [options.maxMuteLists] - Ceiling on mute lists held
|
||||
*/
|
||||
constructor({
|
||||
applicationId,
|
||||
@@ -129,7 +158,11 @@ export class GovernanceRuntime extends Emitter {
|
||||
chunkSize = DEFAULT_CHUNK_SIZE,
|
||||
muteWindowSeconds = 0,
|
||||
verifySignature,
|
||||
trustUnsignedEvents,
|
||||
root,
|
||||
maxCachedDecisions = 5_000,
|
||||
maxReportTargets = 20_000,
|
||||
maxMuteLists = 5_000,
|
||||
}) {
|
||||
super();
|
||||
|
||||
@@ -155,6 +188,10 @@ export class GovernanceRuntime extends Emitter {
|
||||
// rather than pretending state was verified.
|
||||
this.verifySignature = verifySignature;
|
||||
|
||||
// Opt-in escape hatch for local development and tests, where events are
|
||||
// constructed in-process and there is no relay to lie.
|
||||
this.trustUnsignedEvents = trustUnsignedEvents === true;
|
||||
|
||||
// The deployment's own relays, used as a fallback for authors who have
|
||||
// published no NIP-65 list.
|
||||
/** @type {string[]} */
|
||||
@@ -162,8 +199,12 @@ export class GovernanceRuntime extends Emitter {
|
||||
|
||||
this.admin = new GovernanceAdminStore();
|
||||
this.trust = new TrustGraphStore();
|
||||
this.reports = new ReportStore();
|
||||
this.mutes = new TrustedMuteStore({ windowSeconds: muteWindowSeconds, now: this.now });
|
||||
this.reports = new ReportStore({ maxTargets: maxReportTargets });
|
||||
this.mutes = new TrustedMuteStore({
|
||||
windowSeconds: muteWindowSeconds,
|
||||
now: this.now,
|
||||
maxLists: maxMuteLists,
|
||||
});
|
||||
/** @type {PolicyStore} */
|
||||
this.policies = new PolicyStore(policy ?? NEUTRAL_POLICY);
|
||||
this.overrides = new OverrideStore({ now: this.now });
|
||||
@@ -172,8 +213,15 @@ export class GovernanceRuntime extends Emitter {
|
||||
// Seeding it here keeps the storage key stable from construction; without
|
||||
// it, hydration could not find its own cache, because the key is derived
|
||||
// from the root fingerprint the cache would have to supply.
|
||||
if (root) {
|
||||
this.admin.setRoles({ root });
|
||||
// Kept separate from admin.authority.root on purpose. The authority state
|
||||
// can be rewritten by ingested documents; this is the deployment's own
|
||||
// configuration and is what every authorship check compares against, so a
|
||||
// bad ingest cannot move the goalposts for the next one.
|
||||
/** @type {string} */
|
||||
this.configuredRoot = root ? normalizePubkey(root) : "";
|
||||
|
||||
if (this.configuredRoot) {
|
||||
this.admin.setRoles({ root: this.configuredRoot });
|
||||
}
|
||||
|
||||
/** @type {string} */
|
||||
@@ -234,6 +282,10 @@ export class GovernanceRuntime extends Emitter {
|
||||
// for every card is the cost the extraction is supposed to remove.
|
||||
/** @type {Map<string, GovernanceDecision>} */
|
||||
this.decisionCache = new Map();
|
||||
// Without a ceiling this grows for the lifetime of the page: an
|
||||
// infinite-scroll feed evaluates new targets forever and never revisits
|
||||
// most of them. Insertion order gives a cheap approximation of LRU.
|
||||
this.maxCachedDecisions = maxCachedDecisions;
|
||||
|
||||
// The snapshot and its fingerprint are rebuilt only when a store changes.
|
||||
// Deriving them per target walks every report and mute list, which turns a
|
||||
@@ -252,6 +304,8 @@ export class GovernanceRuntime extends Emitter {
|
||||
rejectedSignatures: 0,
|
||||
hydratedFromCache: false,
|
||||
privateMutesApplied: 0,
|
||||
rejectedUnauthorized: 0,
|
||||
rejectedUnverified: 0,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -331,18 +385,32 @@ export class GovernanceRuntime extends Emitter {
|
||||
if (event.kind === CANONICAL_KIND) {
|
||||
const roles = decodeRoles(event);
|
||||
if (roles) {
|
||||
// The roster is the root of the entire trust model: it decides who may
|
||||
// deny whom, and who is protected. Accepting one from an arbitrary
|
||||
// author lets anybody declare themselves root. Only the deployment's
|
||||
// configured root may publish it.
|
||||
if (!this.#isRootAuthored(event)) {
|
||||
this.diagnostics.rejectedUnauthorized += 1;
|
||||
this.emit("rejected", { reason: "roles-not-root", pubkey: event.pubkey });
|
||||
return false;
|
||||
}
|
||||
if (!this.#adminIngestAllowed()) {
|
||||
return false;
|
||||
}
|
||||
this.admin.setRoles(roles);
|
||||
return true;
|
||||
}
|
||||
|
||||
const policyDocument = decodePolicy(event);
|
||||
if (policyDocument) {
|
||||
this.applyRootPolicy(policyDocument, event.pubkey);
|
||||
return true;
|
||||
return this.applyRootPolicy(policyDocument, event.pubkey);
|
||||
}
|
||||
|
||||
const contribution = decodeContribution(event);
|
||||
if (contribution) {
|
||||
if (!this.#adminIngestAllowed()) {
|
||||
return false;
|
||||
}
|
||||
this.admin.upsertContribution(contribution);
|
||||
return true;
|
||||
}
|
||||
@@ -351,6 +419,9 @@ export class GovernanceRuntime extends Emitter {
|
||||
if (event.kind === LEGACY_KIND) {
|
||||
const contribution = decodeLegacyList(event);
|
||||
if (contribution) {
|
||||
if (!this.#adminIngestAllowed()) {
|
||||
return false;
|
||||
}
|
||||
this.admin.upsertContribution(contribution);
|
||||
return true;
|
||||
}
|
||||
@@ -360,6 +431,47 @@ export class GovernanceRuntime extends Emitter {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether an event was authored by the deployment's configured root.
|
||||
*
|
||||
* Falls back to the current authority root only when no root was configured,
|
||||
* which is a development convenience — a deployment with no configured root
|
||||
* has no way to distinguish its administrator from anyone else.
|
||||
*
|
||||
* @param {NostrEvent} event
|
||||
* @returns {boolean}
|
||||
*/
|
||||
#isRootAuthored(event) {
|
||||
const root = this.configuredRoot || this.admin.authority.root;
|
||||
if (!root) {
|
||||
return false;
|
||||
}
|
||||
return normalizePubkey(event?.pubkey ?? "") === root;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether administrative state may be ingested at all.
|
||||
*
|
||||
* Without signature verification, `event.pubkey` is an unauthenticated claim:
|
||||
* a hostile relay can put the root's key on an event the root never wrote, so
|
||||
* an authorship check alone proves nothing. Administrative documents
|
||||
* therefore fail closed unless a verifier is configured, or the application
|
||||
* explicitly opts in for local and test use.
|
||||
*
|
||||
* Trust signals — reports, mutes — are not gated here: they are bounded by
|
||||
* the viewer's own trust graph rather than by authority.
|
||||
*
|
||||
* @returns {boolean}
|
||||
*/
|
||||
#adminIngestAllowed() {
|
||||
if (this.verifySignature || this.trustUnsignedEvents) {
|
||||
return true;
|
||||
}
|
||||
this.diagnostics.rejectedUnverified += 1;
|
||||
this.emit("rejected", { reason: "unverified-administrative-event" });
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply a root-published policy document.
|
||||
*
|
||||
@@ -372,7 +484,7 @@ export class GovernanceRuntime extends Emitter {
|
||||
* @returns {boolean} Whether the policy was accepted
|
||||
*/
|
||||
applyRootPolicy(document, publisher) {
|
||||
const root = this.admin.authority.root;
|
||||
const root = this.configuredRoot || this.admin.authority.root;
|
||||
if (!root || normalizePubkey(publisher) !== root) {
|
||||
this.emit("policy-rejected", { reason: "not-root", publisher });
|
||||
return false;
|
||||
@@ -435,8 +547,19 @@ export class GovernanceRuntime extends Emitter {
|
||||
|
||||
const admin = /** @type {any} */ (await this.storage.read(this.storageKeyFor("admin")));
|
||||
if (admin?.schemaVersion === this.schemaVersion) {
|
||||
// Cache is a performance tier, never an authority tier. Storage is
|
||||
// writable by anything running on this origin, so a cached roster naming
|
||||
// a different root would turn any same-origin XSS into a persistent
|
||||
// moderation takeover. A mismatch means the cache is not ours.
|
||||
const cachedRoot = normalizePubkey(admin.authority?.root ?? "");
|
||||
if (this.configuredRoot && cachedRoot && cachedRoot !== this.configuredRoot) {
|
||||
this.emit("rejected", { reason: "cached-root-mismatch", pubkey: cachedRoot });
|
||||
this.diagnostics.hydratedFromCache = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
if (admin.authority) {
|
||||
this.admin.setRoles(admin.authority);
|
||||
this.admin.setRoles({ ...admin.authority, root: this.configuredRoot || cachedRoot });
|
||||
}
|
||||
if (Array.isArray(admin.contributions)) {
|
||||
this.admin.setContributions(admin.contributions);
|
||||
@@ -463,6 +586,26 @@ export class GovernanceRuntime extends Emitter {
|
||||
return restored;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ingest an event that arrived on a live subscription.
|
||||
*
|
||||
* Subscriptions previously called ingestEvent directly, so state fetched at
|
||||
* load was verified and the identical state arriving live was not — an
|
||||
* attacker only had to wait. Verification is async, so this wraps it.
|
||||
*
|
||||
* @param {NostrEvent} event
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
async #ingestFromSubscription(event) {
|
||||
if (this.verifySignature) {
|
||||
const [verified] = await this.#verified([event]);
|
||||
if (!verified) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return this.ingestEvent(event);
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify a batch of events when a verifier is configured.
|
||||
* @param {NostrEvent[]} events
|
||||
@@ -498,15 +641,35 @@ export class GovernanceRuntime extends Emitter {
|
||||
await this.hydrate();
|
||||
}
|
||||
|
||||
// Narrowed to the exact documents governance uses. An unfiltered query for
|
||||
// kinds 30078 and 30000 asks a relay for every application-data and
|
||||
// people-list event in existence: a self-inflicted denial of service on
|
||||
// page load, and a privacy problem since it pulls unrelated app data.
|
||||
const identifiers = GOVERNANCE_SCOPES.map(
|
||||
(scope) => `${this.namespace}:governance:${scope}:v1`,
|
||||
);
|
||||
|
||||
/** @type {any} */
|
||||
const filter = { kinds: [CANONICAL_KIND, LEGACY_KIND] };
|
||||
const canonicalFilter = {
|
||||
kinds: [CANONICAL_KIND],
|
||||
"#d": identifiers,
|
||||
limit: DEFAULT_ADMIN_LIMIT,
|
||||
};
|
||||
/** @type {any} */
|
||||
const legacyFilter = {
|
||||
kinds: [LEGACY_KIND],
|
||||
"#d": LEGACY_IDENTIFIERS,
|
||||
limit: DEFAULT_ADMIN_LIMIT,
|
||||
};
|
||||
|
||||
if (authors?.length) {
|
||||
filter.authors = authors;
|
||||
canonicalFilter.authors = authors;
|
||||
legacyFilter.authors = authors;
|
||||
}
|
||||
|
||||
let events;
|
||||
try {
|
||||
events = await this.transport.list([filter]);
|
||||
events = await this.transport.list([canonicalFilter, legacyFilter]);
|
||||
} catch (error) {
|
||||
this.stale = true;
|
||||
this.emit("stale", { reason: "relay-unreachable", error: String(error) });
|
||||
@@ -703,7 +866,7 @@ export class GovernanceRuntime extends Emitter {
|
||||
if (!authorChunk.length) continue;
|
||||
opened.push(
|
||||
this.transport.subscribe([{ kinds: [MUTE_LIST_KIND], authors: authorChunk }], {
|
||||
onEvent: (event) => this.ingestEvent(event),
|
||||
onEvent: (event) => void this.#ingestFromSubscription(event),
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -791,7 +954,7 @@ export class GovernanceRuntime extends Emitter {
|
||||
if (!ids.length) continue;
|
||||
opened.push(
|
||||
this.transport.subscribe([{ kinds: [REPORT_KIND], "#e": ids }], {
|
||||
onEvent: (event) => this.ingestEvent(event),
|
||||
onEvent: (event) => void this.#ingestFromSubscription(event),
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -800,7 +963,7 @@ export class GovernanceRuntime extends Emitter {
|
||||
if (!keys.length) continue;
|
||||
opened.push(
|
||||
this.transport.subscribe([{ kinds: [REPORT_KIND], "#p": keys }], {
|
||||
onEvent: (event) => this.ingestEvent(event),
|
||||
onEvent: (event) => void this.#ingestFromSubscription(event),
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -966,6 +1129,17 @@ export class GovernanceRuntime extends Emitter {
|
||||
// Frozen because the cache hands out the same object to every caller; a
|
||||
// consumer mutating a decision would otherwise corrupt later reads.
|
||||
this.decisionCache.set(cacheKey, deepFreeze(decision));
|
||||
|
||||
if (this.decisionCache.size > this.maxCachedDecisions) {
|
||||
// Maps iterate in insertion order, so the first key is the least
|
||||
// recently added — good enough eviction for a cache whose miss cost is
|
||||
// one pure evaluation.
|
||||
const oldest = this.decisionCache.keys().next().value;
|
||||
if (oldest !== undefined) {
|
||||
this.decisionCache.delete(oldest);
|
||||
}
|
||||
}
|
||||
|
||||
return decision;
|
||||
}
|
||||
|
||||
|
||||
@@ -231,10 +231,18 @@ export class TrustGraphStore extends Emitter {
|
||||
* NIP-56 reports, aggregated per target.
|
||||
*/
|
||||
export class ReportStore extends Emitter {
|
||||
constructor() {
|
||||
/**
|
||||
* @param {Object} [options]
|
||||
* @param {number} [options.maxTargets] - Ceiling on distinct targets retained
|
||||
*/
|
||||
constructor({ maxTargets = 0 } = {}) {
|
||||
super();
|
||||
/** @type {Map<string, Map<string, Map<string, number>>>} reports[targetKey][reporter][category] = createdAt */
|
||||
this.reports = new Map();
|
||||
// Relays push reports for targets this viewer may never look at again.
|
||||
// Without a ceiling a long-running session accumulates every target ever
|
||||
// mentioned.
|
||||
this.maxTargets = maxTargets;
|
||||
this.fingerprint = fingerprint({});
|
||||
}
|
||||
|
||||
@@ -275,6 +283,14 @@ export class ReportStore extends Emitter {
|
||||
}
|
||||
|
||||
byCategory.set(report.category, createdAt);
|
||||
|
||||
if (this.maxTargets > 0 && this.reports.size > this.maxTargets) {
|
||||
const oldest = this.reports.keys().next().value;
|
||||
if (oldest !== undefined && oldest !== targetKey) {
|
||||
this.reports.delete(oldest);
|
||||
}
|
||||
}
|
||||
|
||||
this.#emitChange(targetKey);
|
||||
}
|
||||
|
||||
@@ -333,11 +349,14 @@ export class TrustedMuteStore extends Emitter {
|
||||
* @param {Object} [options]
|
||||
* @param {number} [options.windowSeconds] - Mutes older than this are pruned
|
||||
* @param {() => number} [options.now] - Injected clock, unix seconds
|
||||
* @param {number} [options.maxLists] - Ceiling on retained lists; 0 disables eviction
|
||||
*/
|
||||
constructor({ windowSeconds = 0, now = () => 0 } = {}) {
|
||||
constructor({ windowSeconds = 0, now = () => 0, maxLists = 0 } = {}) {
|
||||
super();
|
||||
this.windowSeconds = windowSeconds;
|
||||
this.now = now;
|
||||
/** Ceiling on retained lists; 0 disables eviction. */
|
||||
this.maxLists = maxLists;
|
||||
/** @type {Map<string, { owner: string, updatedAt: number, entries: Array<{ pubkey: string, category?: string }> }>} */
|
||||
this.lists = new Map();
|
||||
this.fingerprint = fingerprint({});
|
||||
@@ -372,6 +391,14 @@ export class TrustedMuteStore extends Emitter {
|
||||
updatedAt: list.updatedAt,
|
||||
entries: list.entries,
|
||||
});
|
||||
|
||||
if (this.maxLists > 0 && this.lists.size > this.maxLists) {
|
||||
const oldest = this.lists.keys().next().value;
|
||||
if (oldest !== undefined && oldest !== list.owner) {
|
||||
this.lists.delete(oldest);
|
||||
}
|
||||
}
|
||||
|
||||
this.#emitChange(Array.from(affected));
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user