diff --git a/packages/core/src/fingerprint.js b/packages/core/src/fingerprint.js index a91d6ba..fc16c48 100644 --- a/packages/core/src/fingerprint.js +++ b/packages/core/src/fingerprint.js @@ -11,15 +11,26 @@ * Canonical JSON: object keys sorted at every depth, so two structurally equal * values always serialize to the same string. * @param {unknown} value + * @param {WeakSet} [seen] - Internal, for cycle detection across recursion * @returns {string} */ -export function canonicalStringify(value) { +export function canonicalStringify(value, seen) { if (value === null || typeof value !== "object") { return JSON.stringify(value) ?? "null"; } + // Cycle guard: a caller could hand us a snapshot fragment that references + // itself, and unguarded recursion is a stack-overflow DoS rather than a + // wrong answer. A repeated object serializes as a sentinel — fingerprints + // only need to be stable and collision-resistant, not reversible. + const visited = seen ?? new WeakSet(); + if (visited.has(value)) { + return '"[cycle]"'; + } + visited.add(value); + if (Array.isArray(value)) { - return `[${value.map((entry) => canonicalStringify(entry)).join(",")}]`; + return `[${value.map((entry) => canonicalStringify(entry, visited)).join(",")}]`; } if (value instanceof Set) { @@ -30,7 +41,7 @@ export function canonicalStringify(value) { const entries = Array.from(value.entries()) .map(([key, entry]) => [String(key), entry]) .sort((a, b) => (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0)); - return `{${entries.map(([key, entry]) => `${JSON.stringify(key)}:${canonicalStringify(entry)}`).join(",")}}`; + return `{${entries.map(([key, entry]) => `${JSON.stringify(key)}:${canonicalStringify(entry, visited)}`).join(",")}}`; } const keys = Object.keys(/** @type {Record} */ (value)).sort(); @@ -40,7 +51,7 @@ export function canonicalStringify(value) { if (entry === undefined) { continue; } - parts.push(`${JSON.stringify(key)}:${canonicalStringify(entry)}`); + parts.push(`${JSON.stringify(key)}:${canonicalStringify(entry, visited)}`); } return `{${parts.join(",")}}`; } diff --git a/packages/nostr/src/mutes.js b/packages/nostr/src/mutes.js index d07db24..b1bc9ed 100644 --- a/packages/nostr/src/mutes.js +++ b/packages/nostr/src/mutes.js @@ -20,6 +20,9 @@ import { getTags } from "./replaceable.js"; /** NIP-51 mute list kind. */ export const MUTE_LIST_KIND = 10000; +/** Largest encrypted mute payload decoded, in bytes of ciphertext. */ +export const MAX_MUTE_CONTENT_BYTES = 128 * 1024; + /** * Resolve the category attached to a mute tag. * @@ -95,6 +98,12 @@ export async function decodePrivateMuteEntries(event, { viewerPubkey, decrypt }) if (!content || typeof decrypt !== "function") { return []; } + // Bound the ciphertext before spending a decrypt and a parse on it. A mute + // list of any realistic size is far under this; anything larger is a resource + // trap, not a real list. + if (content.length > MAX_MUTE_CONTENT_BYTES) { + return []; + } let tags; try { diff --git a/packages/nostr/src/relayList.js b/packages/nostr/src/relayList.js index b168253..23cc799 100644 --- a/packages/nostr/src/relayList.js +++ b/packages/nostr/src/relayList.js @@ -39,9 +39,26 @@ export function normalizeRelayUrl(value) { return ""; } const trimmed = value.trim(); - if (!/^wss?:\/\//i.test(trimmed)) { + + // A relay URL comes from a followed contact's kind:10002 and flows straight + // to `new WebSocket(url)`. Control characters or internal whitespace have no + // legitimate place in a URL and are exactly what a CRLF-injection attempt + // looks like, so reject anything that is not a clean ws(s) URL. The URL + // constructor does the structural validation the old prefix regex skipped. + if (!/^wss?:\/\//i.test(trimmed) || /[\s\u0000-\u001f]/.test(trimmed)) { return ""; } + + let parsed; + try { + parsed = new URL(trimmed); + } catch { + return ""; + } + if ((parsed.protocol !== "wss:" && parsed.protocol !== "ws:") || !parsed.hostname) { + return ""; + } + return trimmed.replace(/\/+$/, "").toLowerCase(); } diff --git a/packages/runtime/src/relayTransport.js b/packages/runtime/src/relayTransport.js index c8694e3..80806c7 100644 --- a/packages/runtime/src/relayTransport.js +++ b/packages/runtime/src/relayTransport.js @@ -105,6 +105,15 @@ export function createRelayTransport(urls, options = {}) { const existing = connections.get(url) ?? { socket: null, ready: false, queue: [], attempts: 0, timer: null }; connections.set(url, existing); + // Defence in depth: even though every internal caller pre-validates relay + // URLs, this is the last point before the socket constructor. A URL with + // control characters or whitespace is malformed by definition and never + // reaches the network. + if (typeof url !== "string" || !/^wss?:\/\//i.test(url) || /[\s\u0000-\u001f]/.test(url)) { + diagnostic("relay url rejected", { url }); + return; + } + let socket; try { socket = new SocketImpl(url); @@ -234,6 +243,9 @@ export function createRelayTransport(urls, options = {}) { if (!url) { continue; } + if (typeof url !== "string" || !/^wss?:\/\//i.test(url) || /[\s\u0000-\u001f]/.test(url)) { + continue; + } if (!connections.has(url)) { connect(url); } diff --git a/packages/widget/src/base.js b/packages/widget/src/base.js index f9b6f45..485f18a 100644 --- a/packages/widget/src/base.js +++ b/packages/widget/src/base.js @@ -172,8 +172,11 @@ export function targetFromAttributes(element) { * Falling back to a plain class keeps the module importable for a * server-rendering host; registration is still a no-op there. */ +/** @type {typeof globalThis.HTMLElement} */ const ElementBase = - typeof globalThis.HTMLElement === "function" ? globalThis.HTMLElement : /** @type {any} */ (class {}); + typeof globalThis.HTMLElement === "function" + ? globalThis.HTMLElement + : /** @type {any} */ (class {}); export class GovernanceElement extends ElementBase { constructor() { diff --git a/packages/widget/src/provider.js b/packages/widget/src/provider.js index 8e518c8..0550a07 100644 --- a/packages/widget/src/provider.js +++ b/packages/widget/src/provider.js @@ -34,8 +34,11 @@ export function requestContext(element) { return detail.context; } +/** @type {typeof globalThis.HTMLElement} */ const ProviderBase = - typeof globalThis.HTMLElement === "function" ? globalThis.HTMLElement : /** @type {any} */ (class {}); + typeof globalThis.HTMLElement === "function" + ? globalThis.HTMLElement + : /** @type {any} */ (class {}); export class BitGateProvider extends ProviderBase { static get observedAttributes() { diff --git a/tests/security/audit-regressions.test.js b/tests/security/audit-regressions.test.js index 60f3d52..629a694 100644 --- a/tests/security/audit-regressions.test.js +++ b/tests/security/audit-regressions.test.js @@ -349,3 +349,64 @@ describe("finding 7: growth is bounded", () => { expect(runtime.mutes.lists.size).toBeLessThanOrEqual(6); }); }); + +describe("second audit: relay url injection", () => { + it("rejects relay urls with control characters or whitespace", async () => { + const { normalizeRelayUrl } = await import("@bitgate/nostr"); + for (const url of ["wss://a\r\nb", "ws:// spaces", "wss://\thost", "wss://", "javascript:alert(1)"]) { + expect(normalizeRelayUrl(url), url).toBe(""); + } + expect(normalizeRelayUrl("wss://ok.example/")).toBe("wss://ok.example"); + }); + + it("drops a malicious contact's crlf relay from an outbox grouping", async () => { + const { groupAuthorsByWriteRelay } = await import("@bitgate/nostr"); + const pubkey = "d4".repeat(32); + const lists = new Map([ + [pubkey, { pubkey, updatedAt: NOW, read: [], write: ["wss://evil\r\nInjected: x", "wss://good.example"] }], + ]); + const grouped = groupAuthorsByWriteRelay([pubkey], lists); + expect([...grouped.keys()]).toEqual(["wss://good.example"]); + }); +}); + +describe("second audit: fingerprint cycle safety", () => { + it("does not overflow on a self-referential value", async () => { + const { canonicalStringify, fingerprint } = await import("@bitgate/core"); + const cyclic = { a: 1 }; + cyclic.self = cyclic; + expect(() => canonicalStringify(cyclic)).not.toThrow(); + expect(() => fingerprint(cyclic)).not.toThrow(); + }); + + it("still distinguishes genuinely different structures", async () => { + const { fingerprint } = await import("@bitgate/core"); + expect(fingerprint({ a: 1 })).not.toBe(fingerprint({ a: 2 })); + }); +}); + +describe("second audit: private mute payload is bounded", () => { + it("refuses an oversized encrypted payload before parsing", async () => { + const { decodePrivateMuteEntries, MUTE_LIST_KIND } = await import("@bitgate/nostr"); + const viewer = "d4".repeat(32); + const event = { + id: "a".repeat(64), + pubkey: viewer, + kind: MUTE_LIST_KIND, + created_at: 1, + tags: [], + content: "x".repeat(200 * 1024), + }; + let decryptCalled = false; + const entries = await decodePrivateMuteEntries(event, { + viewerPubkey: viewer, + decrypt: async () => { + decryptCalled = true; + return "[]"; + }, + }); + expect(entries).toEqual([]); + // The bound is checked before spending a decrypt on it. + expect(decryptCalled).toBe(false); + }); +});