|
| 1 | +# SECURITY.md — audit record |
| 2 | + |
| 3 | +A four-axis adversarial review of all 33 plugins (2026-07), and what it |
| 4 | +found. Axes: authentication/authorization, SSRF/path/parser input, |
| 5 | +resource-safety/DoS/lifecycle, and correctness/protocol conformance. Each |
| 6 | +reviewer read every `plugin.js` and cross-checked findings against |
| 7 | +[NOTES.md](./NOTES.md)/[REPORT.md](./REPORT.md) so documented limitations |
| 8 | +were not reported as bugs. |
| 9 | + |
| 10 | +**Headline:** the fleet held up well. The dominant pattern — |
| 11 | +resolve identity with `api.auth.getAgent`, forward the client's |
| 12 | +`Authorization` on every loopback call, and let the host's WAC decide — |
| 13 | +is applied correctly and consistently across the whole data plane. Real |
| 14 | +defects clustered exactly where a plugin *invented its own* behavior |
| 15 | +instead of deferring to WAC (activitypub's unauthenticated federation |
| 16 | +surface, sparql's user-supplied regex). All confirmed findings are |
| 17 | +fixed, each with a regression test; the suite went 374 → 386 tests. |
| 18 | + |
| 19 | +## Fixed |
| 20 | + |
| 21 | +| Severity | Plugin(s) | Finding | Fix | Commit | |
| 22 | +|---|---|---|---|---| |
| 23 | +| High | activitypub | Unauthenticated inbox fetched an attacker-controlled `actor`/`inbox` URL with `redirect: follow` and no gate → SSRF pivot (e.g. `169.254.169.254`); unbounded per-user state growth; non-atomic write | `gatedFetch` (corsproxy's private-IP guard + DNS `resolvesToPublic`, fail-closed, manual per-hop redirect re-validation); inbox trimmed to `maxInbox`, followers deduped; atomic state write; bodies consumed; outbox walk capped | `03e4db5` | |
| 24 | +| High | sparql | `FILTER(REGEX(...))` compiled a user pattern and ran it on pod-plantable strings with no bound → ReDoS event-loop hang | Reject patterns over `maxRegexLength` (512) and catastrophic nested-quantifier shapes; truncate tested input to `maxRegexInput` (10k) | `f1255a7` | |
| 25 | +| High | caldav, carddav | free-busy/query/multiget/PROPFIND walked every member with a full-body buffer each, uncapped → DoS amplification | `config.maxResources` cap (default 10k) truncates the member/href list before the walk; oversized → truncated-but-valid 207 | `642dec2` | |
| 26 | +| Med | otp, capability, shortlink | Direct `writeFileSync` of the whole table → a crash mid-write truncates it; loader swallows the parse error into `{}` (capability: a revoked token silently revives) | Atomic write (unique temp file + `renameSync`) | `ca52373` | |
| 27 | +| Med | jmap | `Email/set` create omitted server-set props (`blobId`/`threadId`/`size`) required by RFC 8621 §4.6 | Echo them in the create response (blobId = sha256 of stored JSON, threadId = id, size = byte length) | `e54aba2` | |
| 28 | +| Low-Med | gitscratch | Auth gate required *an* agent but never checked it against the repo creator → any authed user could push to another's repo | Opt-in `config.owned` binds a repo to its first materializer (default stays shared) | `a8847a3` | |
| 29 | +| Low | rss | Atom output missing the RFC 4287 §4.1.1-required `<author>` | Feed-level `<author>` from `config.author`/title | `e54aba2` | |
| 30 | +| Low | remotestorage | Forwarded host `content-length` over a fetch-decoded stream (latent framing bug) | Drop it, matching the webdav sibling | `e54aba2` | |
| 31 | +| Low | terminal | Query-token compare early-exited on length mismatch (timing side-channel on token length) | Constant-time sha256 + `timingSafeEqual` | `e54aba2` | |
| 32 | + |
| 33 | +## Accepted risk / deferred (documented, not fixed) |
| 34 | + |
| 35 | +- **activitypub inbound signature verification is still Phase 2.** The |
| 36 | + inbox remains unauthenticated by design; the SSRF gate + state caps are |
| 37 | + the mitigation. Full HTTP-Signature verification is a larger feature, |
| 38 | + tracked as a known limitation in activitypub/README.md. |
| 39 | +- **DNS-rebinding TOCTOU** in `gatedFetch` (activitypub) and `corsproxy/`: |
| 40 | + the address is validated at `dns.lookup` time, but `fetch` resolves |
| 41 | + again independently, so a hostname that flips between the two resolutions |
| 42 | + can slip past. Same residual both places; closing it needs pinning the |
| 43 | + resolved IP into the connection (no clean dep-free seam today). |
| 44 | +- **s3 ListObjectsV2 fetches every object to compute its md5 ETag** |
| 45 | + (`s3/`): the crawl is capped (`WALK_MAX_RESOURCES`) and cycle-safe, but a |
| 46 | + single list of large objects buffers each in turn. Performance, not a |
| 47 | + vulnerability; a write-time metadata cache (the `api.events` seam) would |
| 48 | + fix it properly. |
| 49 | +- **relay rewrites its whole event file per stored EVENT** (`relay/`): |
| 50 | + blocking O(N) I/O on the event loop under sustained traffic (per-socket |
| 51 | + rate-limited to 60/min). Bounded by `maxEvents`; an append-only log is |
| 52 | + the real fix. Performance, deferred. |
| 53 | +- **No per-resource body-size ceiling** on buffered loopback reads (rss, |
| 54 | + backup, s3, caldav/carddav): one-at-a-time bounds it to the largest |
| 55 | + single resource, but a multi-GB pod file could OOM. `corsproxy/` is the |
| 56 | + counter-example (streamed with `maxBodyBytes`). A shared capped-read |
| 57 | + helper would generalize the fix; deferred. |
| 58 | +- **Read-modify-write TOCTOU** in the pluginDir JSON writers (otp, |
| 59 | + capability, shortlink, activitypub): the *write* is now atomic, but two |
| 60 | + concurrent request handlers can still interleave read→modify→write and |
| 61 | + lose one update. Distinct from the atomicity fix above and from core |
| 62 | + [#600](https://github.com/JavaScriptSolidServer/JavaScriptSolidServer/issues/600) |
| 63 | + (the host's own conditional-write non-atomicity). Low impact at scratch |
| 64 | + scale; a per-key lock is the fix. |
| 65 | + |
| 66 | +## What was checked and found solid |
| 67 | + |
| 68 | +- **WAC deferral**: every data-plane plugin forwards the caller's |
| 69 | + `Authorization` on loopback and lets WAC decide; none forward it |
| 70 | + off-host. `corsproxy/` strips pod credentials from upstream and drops |
| 71 | + them on cross-origin redirect. Liveness probes (metrics/dashboard/admin) |
| 72 | + deliberately send no auth and see only public responses. |
| 73 | +- **Token/secret handling**: capability, otp, s3, metrics all verify the |
| 74 | + signature before trusting the payload, compare with `timingSafeEqual` |
| 75 | + over equal-length buffers, and gate JSON parse behind the MAC. No |
| 76 | + `Math.random` in any security path; secrets from `crypto.randomBytes`. |
| 77 | +- **Traversal / containment**: webfinger, didweb, nip05 reject `..`/dot |
| 78 | + segments before `path.join`; admin's podsRoot walk is realpath-contained |
| 79 | + and skips symlinks; the DAV/s3/backup/remotestorage path handlers reject |
| 80 | + traversal and rely on host WAC over loopback. |
| 81 | +- **Parsers**: the backup ustar writer, oembed image-dimension parsers, |
| 82 | + caldav iCal RRULE expansion (capped), and the sparql tokenizer are |
| 83 | + bounds-checked and always advance (no over-read, no infinite loop); s3 |
| 84 | + and oembed build XML with escaping and parse no untrusted XML (no XXE). |
| 85 | +- **XSS**: oembed refuses rich HTML and escapes XML; dashboard and admin |
| 86 | + inject values only via client-side `.textContent` (config via |
| 87 | + `JSON.stringify` into a script tag), so pod-derived data never lands as |
| 88 | + markup. |
| 89 | +- **Lifecycle**: the WebSocket plugins (relay, webrtc, terminal, tunnel, |
| 90 | + notifications) track sockets and clean up on close/error with a working |
| 91 | + `deactivate()`; timers/watchers (metrics, gitscratch, search, |
| 92 | + notifications) are cleared on teardown. |
| 93 | + |
| 94 | +## Reporting |
| 95 | + |
| 96 | +This is an experimental research repo, not a production deployment. If you |
| 97 | +find a security issue, open an issue on the repo (or, for the core server, |
| 98 | +on JavaScriptSolidServer/JavaScriptSolidServer). None of these plugins |
| 99 | +should be run on a public deployment without the operator reviewing the |
| 100 | +accepted-risk items above. |
0 commit comments