Changeset 3dd99d3 in Klonkt

Timestamp:

06/25/2026 09:52:25 AM (3 months ago)

Author:

Robin Genis <roboburr@…>

Branches:

main

Children:

c648b04

Parents:

4b5223f

Message:

harden(fediverse): SSRF guard, remote-URL XSS scheme-guard, scoped Delete, gate-by-default + queue fixes

From a 3-agent hardening review of this session's fediverse code:

• SSRF: all outbound fetches (deliver/fetchActor/webfingerResolve) now go through safeFetch — http(s)-only, rejects hosts resolving to private/loopback/link-local ranges on the initial host AND every redirect hop (redirect:manual), + actor-doc size cap. Blocks inbox-driven SSRF to cloud-metadata/internal services.
• Stored XSS: remote actor url/icon, timeline media + author urls, and remote-note images/object_uri are now run through an http(s) scheme-guard before storage, so a malicious actor can't smuggle javascript:/data: into owner-only-rendered href/src.
• Cross-actor Delete: inbound Delete is now scoped to the signing actor (can't wipe another actor's replies/timeline rows).
• Gate-by-default: Add/Remove/Update added to the signature-enforced activity list.
• Delivery queue: re-entrancy guard (30 rows x 8s can exceed the 60s tick -> no double-delivery) + backoff off-by-one fix (1-min first retry no longer skipped).
• Scheduler: delete-before-insert on FTS so a re-flipped post has no duplicate row.
• /meldingen: don't mark-seen for a viewer (GET-side mutation the global guard misses).
• Activity ids get a random suffix to avoid same-millisecond collisions.

Co-Authored-By: Claude <noreply@…>

Location:

src

Files:

• routes/posts.js (modified) (2 diffs)
• services/ActivityPubService.js (modified) (20 diffs)
• services/Scheduler.js (modified) (1 diff)

src/routes/posts.js

@@ -7 +7 @@
 import ejs from 'ejs';
 import db from '../config/database.js';
-import { requireAuth, requireSiteManager } from '../middleware/auth.js';
+import { requireAuth, requireSiteManager, isViewer } from '../middleware/auth.js';
 import { renderPage } from '../middleware/render.js';
 import { recordPageview, recordPostView } from '../services/StatsService.js';
@@ -591 +591 @@
   const site = res.locals.site;
   const items = site ? ActivityPubService.getNotifications(site.slug, 80) : [];
-  if (site) ActivityPubService.markNotificationsSeen(site.slug); // viewing = seen → clears the bell badge
+  // viewing = seen → clears the bell badge. A viewer (kijker) may look but must not
+  // mutate state (the global write-guard only catches non-GET, not this GET-side effect).
+  if (site && !isViewer(req.session.user)) ActivityPubService.markNotificationsSeen(site.slug);
   renderPage(req, res, 'pages/fedi-notifications', { pageTitle: 'Meldingen', bodyClass: 'on-special', items });
 });

src/services/ActivityPubService.js

@@ -17 +17 @@
  */
 import crypto from 'crypto';
+import dns from 'dns';
+import net from 'net';
 import db from '../config/database.js';
 import HtmlSanitizerService from './HtmlSanitizerService.js';
 
 const PUBLIC = 'https://www.w3.org/ns/activitystreams#Public';
+
+// Short random suffix so two activity ids minted in the same millisecond (e.g.
+// parallel saves) don't collide and get deduped by a receiver.
+const rid = () => crypto.randomBytes(4).toString('hex');
+
+// Keep only http(s) URLs — drops javascript:/data:/etc so a remote actor can't
+// smuggle a dangerous scheme into a stored href/src (rendered in owner-only views).
+const safeUrl = (u) => { const s = String(u == null ? '' : u).trim(); return /^https?:\/\//i.test(s) ? s : ''; };
+
+// ── SSRF guard for outbound fetches ───────────────────────────────
+// Remote URLs (actor/keyId/webfinger/inbox/inReplyTo) are attacker-controlled, so
+// every outbound fetch must refuse hosts that resolve to private/loopback ranges
+// (cloud metadata, internal services) — on the initial host AND each redirect hop.
+function isBlockedIp(ip) {
+  if (!ip) return true;
+  const v = net.isIP(ip);
+  if (v === 4) {
+    const o = ip.split('.').map(Number);
+    return o[0] === 127 || o[0] === 10 || o[0] === 0
+      || (o[0] === 172 && o[1] >= 16 && o[1] <= 31)
+      || (o[0] === 192 && o[1] === 168)
+      || (o[0] === 169 && o[1] === 254)
+      || (o[0] === 100 && o[1] >= 64 && o[1] <= 127); // CGNAT
+  }
+  if (v === 6) {
+    const s = ip.toLowerCase().replace(/^\[|\]$/g, '');
+    return s === '::1' || s === '::' || s.startsWith('fc') || s.startsWith('fd') || s.startsWith('fe80')
+      || s.startsWith('::ffff:127.') || s.startsWith('::ffff:10.') || s.startsWith('::ffff:192.168.')
+      || s.startsWith('::ffff:169.254.') || s.startsWith('::ffff:172.');
+  }
+  return true; // not an IP literal we recognise → refuse
+}
+async function assertPublicHost(hostname) {
+  if (net.isIP(hostname)) { if (isBlockedIp(hostname)) throw new Error('ssrf-blocked-ip'); return; }
+  const addrs = await dns.promises.lookup(hostname, { all: true });
+  if (!addrs.length || addrs.some((a) => isBlockedIp(a.address))) throw new Error('ssrf-blocked-host');
+}
+async function safeFetch(url, opts = {}, maxRedirects = 3) {
+  let target = url;
+  for (let hop = 0; ; hop++) {
+    const u = new URL(target); // throws on malformed → caller's catch
+    if (u.protocol !== 'https:' && u.protocol !== 'http:') throw new Error('ssrf-bad-scheme');
+    await assertPublicHost(u.hostname);
+    const r = await fetch(target, { ...opts, redirect: 'manual', signal: AbortSignal.timeout(8000) });
+    const loc = (r.status >= 300 && r.status < 400) ? r.headers.get('location') : null;
+    if (loc && hop < maxRedirects) { target = new URL(loc, target).toString(); continue; }
+    return r;
+  }
+}
 const MAX_OUTBOX = 20;
 // Cache-buster for the music listen-link → forces Mastodon to re-crawl a FRESH
@@ -323 +374 @@
     name: (doc && (doc.name || doc.preferredUsername)) || handle,
     handle,
-    url: (doc && (doc.url || doc.id)) || actorUri,
-    icon: icon || null,
+    url: safeUrl((doc && (doc.url || doc.id)) || actorUri) || null,
+    icon: safeUrl(icon) || null,
   };
 }
@@ -406 +457 @@
   const signature = crypto.sign('sha256', Buffer.from(signingString), privatePem).toString('base64');
   const sig = `keyId="${keyId}",algorithm="rsa-sha256",headers="(request-target) host date digest",signature="${signature}"`;
-  const r = await fetch(inboxUrl, {
+  const r = await safeFetch(inboxUrl, {
     method: 'POST',
     headers: { 'Content-Type': 'application/activity+json', Accept: 'application/activity+json', Date: date, Digest: digest, Signature: sig },
     body,
-    signal: AbortSignal.timeout(8000),
   });
   return r.status;
@@ -417 +467 @@
 export async function fetchActor(url) {
   try {
-    const r = await fetch(url, { headers: { Accept: 'application/activity+json' }, redirect: 'follow', signal: AbortSignal.timeout(8000) });
+    const r = await safeFetch(url, { headers: { Accept: 'application/activity+json' } });
     if (!r.ok) return null;
+    const len = Number(r.headers.get('content-length') || 0);
+    if (len > 2_000_000) return null; // refuse oversized actor docs
     return await r.json();
   } catch { return null; }
@@ -450 +502 @@
   enqueueDelivery(slug, inbox, activity);
 }
+let _processingDeliv = false;
 export async function processDeliveryQueue() {
-  let rows;
-  try { rows = deliveryStmts().due.all(); } catch { return; }
-  if (!rows || !rows.length) return;
-  const base = (process.env.PUBLIC_BASE_URL || '').replace(/\/+$/, '');
-  for (const row of rows) {
-    let ok = false;
-    try {
-      const keys = getOrCreateKeys(row.slug);
-      const st = await deliver(row.inbox, JSON.parse(row.body), `${actorId(base, row.slug)}#main-key`, keys.private_pem);
-      ok = st >= 200 && st < 300;
-    } catch { ok = false; }
-    if (ok) { deliveryStmts().del.run(row.id); continue; }
-    const attempts = row.attempts + 1;
-    if (attempts >= DELIVERY_MAX_ATTEMPTS) { deliveryStmts().del.run(row.id); console.warn('[AP] delivery gave up after', attempts, 'tries →', row.inbox); continue; }
-    const mins = DELIVERY_BACKOFF_MIN[Math.min(attempts, DELIVERY_BACKOFF_MIN.length - 1)];
-    deliveryStmts().bump.run(attempts, new Date(Date.now() + mins * 60000).toISOString(), row.id);
-  }
+  if (_processingDeliv) return; // re-entrancy guard: 30 rows × 8s can exceed the 60s tick → no double-delivery
+  _processingDeliv = true;
+  try {
+    let rows;
+    try { rows = deliveryStmts().due.all(); } catch { return; }
+    if (!rows || !rows.length) return;
+    const base = (process.env.PUBLIC_BASE_URL || '').replace(/\/+$/, '');
+    for (const row of rows) {
+      let ok = false;
+      try {
+        const keys = getOrCreateKeys(row.slug);
+        const st = await deliver(row.inbox, JSON.parse(row.body), `${actorId(base, row.slug)}#main-key`, keys.private_pem);
+        ok = st >= 200 && st < 300;
+      } catch { ok = false; }
+      if (ok) { deliveryStmts().del.run(row.id); continue; }
+      const attempts = row.attempts + 1;
+      if (attempts >= DELIVERY_MAX_ATTEMPTS) { deliveryStmts().del.run(row.id); console.warn('[AP] delivery gave up after', attempts, 'tries →', row.inbox); continue; }
+      // Index the backoff on the CURRENT attempt count (row.attempts) so the first
+      // retry uses the 1-min tier instead of skipping it.
+      const mins = DELIVERY_BACKOFF_MIN[Math.min(row.attempts, DELIVERY_BACKOFF_MIN.length - 1)];
+      deliveryStmts().bump.run(attempts, new Date(Date.now() + mins * 60000).toISOString(), row.id);
+    }
+  } finally { _processingDeliv = false; }
 }
 let _delivTimer = null;
@@ -512 +571 @@
   // Blocked actor/domain → silently drop (202, don't reveal the block).
   if (claimedActor && isBlockedAny(claimedActor)) { console.log('[AP] inbox dropped (blocked)', claimedActor); return 202; }
-  const GATED = ['Create', 'Like', 'Announce', 'Follow', 'Delete', 'Undo', 'Accept', 'Reject'];
+  const GATED = ['Create', 'Like', 'Announce', 'Follow', 'Delete', 'Undo', 'Accept', 'Reject', 'Add', 'Remove', 'Update'];
   if (GATED.includes(type)) {
     if (!verified || !claimedActor || verified.id !== claimedActor) {
@@ -529 +588 @@
     const me = actorId(base, slug);
     const keys = getOrCreateKeys(slug);
-    const accept = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${me}#accept-${Date.now()}`, type: 'Accept', actor: me, object: act };
+    const accept = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${me}#accept-${Date.now()}-${rid()}`, type: 'Accept', actor: me, object: act };
     deliver(remote.inbox, accept, `${me}#main-key`, keys.private_pem).catch((e) => console.warn('[AP] Accept delivery failed:', e.message));
     // Auto-backfill: send the new follower our recent posts as Create so their
@@ -577 +636 @@
         const ai = actorInfo(await resolveActor(actorUri), actorUri);
         const html = HtmlSanitizerService.sanitize(o.content || '');
-        const media = JSON.stringify((Array.isArray(o.attachment) ? o.attachment : []).filter((a) => a && a.url).map((a) => ({ url: a.url, type: a.mediaType || '' })));
+        const media = JSON.stringify((Array.isArray(o.attachment) ? o.attachment : []).map((a) => ({ url: safeUrl(a && a.url), type: (a && a.mediaType) || '' })).filter((m) => m.url));
         for (const s of subs) tlStmts().ins.run(o.id, s.slug, actorUri, ai.name, ai.handle, ai.icon, ai.url, html, o.url || null, o.published || null, media);
         console.log('[AP] timeline +', actorUri, 'x' + subs.length);
@@ -596 +655 @@
   if (type === 'Delete') {
     // A remote note was deleted upstream → drop it from replies AND the timeline.
+    // Scope to the SIGNING actor so actor B can't delete actor A's content (the
+    // signature gate guarantees claimedActor == the verified signer here).
     const oid = typeof act.object === 'string' ? act.object : (act.object && act.object.id);
-    if (oid) { iStmts().delReply.run(oid); try { tlStmts().del.run(oid); } catch { /* ignore */ } }
+    if (oid && claimedActor) {
+      try { db.prepare('DELETE FROM ap_interactions WHERE object_uri = ? AND actor_uri = ?').run(oid, claimedActor); } catch { /* ignore */ }
+      try { db.prepare('DELETE FROM ap_timeline WHERE id = ? AND author_uri = ?').run(oid, claimedActor); } catch { /* ignore */ }
+    }
     return 202;
   }
@@ -665 +729 @@
   const del = {
     '@context': 'https://www.w3.org/ns/activitystreams',
-    id: `${nid}#delete-${Date.now()}`,
+    id: `${nid}#delete-${Date.now()}-${rid()}`,
     type: 'Delete',
     actor: me,
@@ -688 +752 @@
   const update = {
     '@context': 'https://www.w3.org/ns/activitystreams',
-    id: `${noteId(base, post.id)}#update-${Date.now()}`,
+    id: `${noteId(base, post.id)}#update-${Date.now()}-${rid()}`,
     type: 'Update', actor: me, to: [PUBLIC], cc: [`${me}/followers`],
     object: note,
@@ -708 +772 @@
   const update = {
     '@context': ['https://www.w3.org/ns/activitystreams', 'https://w3id.org/security/v1'],
-    id: `${me}#update-${Date.now()}`,
+    id: `${me}#update-${Date.now()}-${rid()}`,
     type: 'Update', actor: me, to: [PUBLIC], cc: [`${me}/followers`],
     object: buildActor(base, site),
@@ -741 +805 @@
   // 1. Remove every current pin so Mastodon can recreate them in order.
   for (const id of removeIds) {
-    const rm = { '@context': AS, id: `${me}#rm-${id}-${Date.now()}`, type: 'Remove', actor: me, object: note(id), target: featured, to: [PUBLIC] };
+    const rm = { '@context': AS, id: `${me}#rm-${id}-${Date.now()}-${rid()}`, type: 'Remove', actor: me, object: note(id), target: featured, to: [PUBLIC] };
     for (const inbox of inboxes) deliver(inbox, rm, keyId, keys.private_pem).catch(() => { /* best-effort */ });
   }
@@ -748 +812 @@
   // 2. Add in rank-DESC order, gaps so each StatusPin gets an increasing created_at.
   for (const p of pinned) {
-    const add = { '@context': AS, id: `${me}#add-${p.id}-${Date.now()}`, type: 'Add', actor: me, object: note(p.id), target: featured, to: [PUBLIC], cc: [`${me}/followers`] };
+    const add = { '@context': AS, id: `${me}#add-${p.id}-${Date.now()}-${rid()}`, type: 'Add', actor: me, object: note(p.id), target: featured, to: [PUBLIC], cc: [`${me}/followers`] };
     for (const inbox of inboxes) deliver(inbox, add, keyId, keys.private_pem).catch(() => { /* best-effort */ });
     await new Promise((r) => setTimeout(r, 2000));
@@ -865 +929 @@
   const images = (Array.isArray(note.attachment) ? note.attachment : [])
     .filter((a) => a && a.url && (!a.mediaType || /^image\//i.test(a.mediaType)))
-    .map((a) => a.url);
+    .map((a) => safeUrl(a.url)).filter(Boolean);
   return {
-    object_uri: note.id,
+    object_uri: safeUrl(note.id) || note.id,
     actor_uri: actorUri,
     actor_url: ai.url,
@@ -895 +959 @@
     const me = actorId(base, site.slug);
     const nid = noteId(base, row.id);
-    const del = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${nid}#delete-${Date.now()}`, type: 'Delete', actor: me, to: [PUBLIC], object: { id: nid, type: 'Tombstone' } };
+    const del = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${nid}#delete-${Date.now()}-${rid()}`, type: 'Delete', actor: me, to: [PUBLIC], object: { id: nid, type: 'Tombstone' } };
     const keys = getOrCreateKeys(site.slug);
     const inboxes = new Set();
@@ -914 +978 @@
   const acct = `${parts[0]}@${parts[1]}`;
   try {
-    const r = await fetch(`https://${parts[1]}/.well-known/webfinger?resource=acct:${encodeURIComponent(acct)}`,
-      { headers: { Accept: 'application/jrd+json, application/json' }, redirect: 'follow', signal: AbortSignal.timeout(8000) });
+    const r = await safeFetch(`https://${parts[1]}/.well-known/webfinger?resource=acct:${encodeURIComponent(acct)}`,
+      { headers: { Accept: 'application/jrd+json, application/json' } });
     if (!r.ok) return null;
     const jrd = await r.json();
     const link = (jrd.links || []).find((l) => l.rel === 'self' && /activity\+json|ld\+json/.test(l.type || ''));
-    return link ? link.href : null;
+    return safeUrl(link ? link.href : '') || null;
   } catch { return null; }
 }
@@ -958 +1022 @@
   const me = actorId(base, site.slug);
   const keys = getOrCreateKeys(site.slug);
-  const followId = `${me}#follow-${Date.now()}`;
+  const followId = `${me}#follow-${Date.now()}-${rid()}`;
   fwStmts().ins.run(site.slug, actor.id, ai.handle, ai.name, ai.icon, ai.url, actor.inbox, followId, 'pending');
   const follow = { '@context': 'https://www.w3.org/ns/activitystreams', id: followId, type: 'Follow', actor: me, object: actor.id };
@@ -973 +1037 @@
   const row = fwStmts().one.get(site.slug, actorUri);
   if (row && row.inbox) {
-    const undo = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${me}#unfollow-${Date.now()}`, type: 'Undo', actor: me, object: { id: row.follow_id || `${me}#follow`, type: 'Follow', actor: me, object: actorUri } };
+    const undo = { '@context': 'https://www.w3.org/ns/activitystreams', id: `${me}#unfollow-${Date.now()}-${rid()}`, type: 'Undo', actor: me, object: { id: row.follow_id || `${me}#follow`, type: 'Follow', actor: me, object: actorUri } };
     try { await deliver(row.inbox, undo, `${me}#main-key`, keys.private_pem); } catch { /* best-effort */ }
   }
@@ -989 +1053 @@
   const act = {
     '@context': 'https://www.w3.org/ns/activitystreams',
-    id: `${me}#${type.toLowerCase()}-${Date.now()}`,
+    id: `${me}#${type.toLowerCase()}-${Date.now()}-${rid()}`,
     type, actor: me, object: targetNoteId,
   };

src/services/Scheduler.js

@@ -24 +24 @@
       "UPDATE posts SET status = 'published', published_at = COALESCE(published_at, publish_at, CURRENT_TIMESTAMP) WHERE id = ?"
     );
+    const ftsDel = db.prepare('DELETE FROM posts_fts WHERE post_id = ?');
     const fts = db.prepare('INSERT INTO posts_fts(content, title, author, post_id) VALUES (?, ?, ?, ?)');
     const siteStmt = db.prepare('SELECT * FROM sites WHERE id = ?');
     for (const p of due) {
       upd.run(p.id);
-      try { fts.run(HtmlSanitizerService.toPlainText(p.content || ''), p.title || '', p.username || '', p.id); } catch { /* FTS failure is non-fatal */ }
+      // Delete-before-insert so a re-scheduled (previously published) post doesn't
+      // get a duplicate FTS row → duplicate search hits.
+      try { ftsDel.run(p.id); fts.run(HtmlSanitizerService.toPlainText(p.content || ''), p.title || '', p.username || '', p.id); } catch { /* FTS failure is non-fatal */ }
       // ActivityPub: federate the now-published post to followers.
       if (!p.fan_only) {