9e6756f678
Backfills transcripts that existed only locally in ohm-infra: - 0019.1/.2/.3 - UX-polish wave subagent transcripts (driver 0019.0 never finalized) - 0026.1-.9 - security-audit-0026 subagent transcripts (driver abandoned/closed-out by 0068; audit drove published 0030 remediation) - 0073.0 - PPE/progressive-delivery + engineering-handbook session, originally drafted as a duplicate 0046.0; reassigned next free number (0072 taken by a concurrent session) sessions.json: add 0019/0073 titles, update 0026 title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
137 lines
7.5 KiB
Markdown
137 lines
7.5 KiB
Markdown
# SESSION-0026.2 — rfc-app Injection & Input-Handling Security Audit
|
|
|
|
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
|
Subsession: 0026.2 (read-only audit leg of security-audit driver session 0026)
|
|
Date: 2026-05-28 (PST)
|
|
Target: rfc-app at /Users/benstull/git/rfc-app, tag v0.24.0 (HEAD 28015ed)
|
|
Scope: SQL injection, XSS, SSRF, path traversal, email/markdown sanitization.
|
|
Mode: READ-ONLY. No edits, no deploys, no git mutations. No secret bytes recorded.
|
|
|
|
## Pre-state
|
|
|
|
Driver assigned the injection & input-handling surface. Repo confirmed at
|
|
tag v0.24.0. Backend is FastAPI + SQLite (backend/app/*.py); frontend is
|
|
React 19 + Vite (frontend/src). Markdown via `marked` v18; no sanitizer
|
|
library in frontend/package.json dependencies.
|
|
|
|
## Turn-by-turn arc
|
|
|
|
1. Enumerated backend modules and grepped all 321 `execute*` call sites for
|
|
f-string / `.format` / `%` / concat SQL. Pulled the ~16 dynamic-SQL hits.
|
|
|
|
2. SQL injection — examined every dynamic-SQL site:
|
|
- api_notifications.py:274 (`UPDATE users SET {', '.join(sets)}`) — `sets`
|
|
contains only hardcoded column-name literals; values parameterized via
|
|
`args`. Safe.
|
|
- api_notifications.py:479/483 (`UPDATE users SET {column} = ...`) —
|
|
`column` from static `_CATEGORY_COLUMN` allowlist. Safe.
|
|
- api_admin.py:413, notify.py:535, digest.py:201, email.py:610,
|
|
notify.py:942/964/1000 — all IN-clauses built via `",".join("?" * len)`,
|
|
the correct parameterized-IN idiom; values in the tuple. Safe.
|
|
- notify.py:638/643 — `{now}` is the literal `datetime('now')`, not input.
|
|
- api_admin.py:629/711/754 — `WHERE` built from `clauses` lists holding
|
|
only fixed `col = ?` strings; user values parameterized; `limit` is
|
|
FastAPI `int` with ge/le bounds. Safe.
|
|
- db.py — connection/migration layer; migrations read trusted on-disk .sql.
|
|
- Swept for dynamic ORDER BY / LIMIT / table-name injection: none found.
|
|
Verdict: parameterization is consistent and correct across the backend.
|
|
|
|
3. XSS — found the markdown pipeline:
|
|
- frontend/package.json has NO DOMPurify / rehype-sanitize / any sanitizer.
|
|
- MarkdownPreview.jsx:101-102 — `previewMarked.parse(content)` →
|
|
`hostRef.current.innerHTML = html`. The custom `Marked` instance only
|
|
overrides the ```mermaid code-fence renderer; everything else is marked
|
|
default (raw HTML passthrough). `escapeHtml` is applied ONLY to the
|
|
mermaid placeholder text and error text, NOT the document body.
|
|
- ProposalView.jsx:164 & 172 — `dangerouslySetInnerHTML` with the GLOBAL
|
|
unconfigured `marked.parse(data.entry?.body)` / `proposed_use_case`.
|
|
- Content sources: RFCView passes RFC body (previewContent/editorContent)
|
|
into MarkdownPreview; DocsSessionTranscript / DocsSpec / DocsUserGuide /
|
|
Philosophy / DocsSessionsAbout/Index also feed body through it. RFC body
|
|
and proposal body/use-case are free-text authored by any invited user.
|
|
- Backend: no sanitization on write (api.py — only Field max_length caps).
|
|
- Self-refutation step: ran `marked.parse` on
|
|
`<img src=x onerror=...> <script> [xss](javascript:...)`. Output passes
|
|
all three through verbatim. With innerHTML/dangerouslySetInnerHTML the
|
|
`<img onerror>`, `<svg onload>`, event handlers and `javascript:` links
|
|
execute (the bare `<script>` won't via innerHTML, but the rest do).
|
|
Confirmed stored XSS. Killed nothing — sink reachable, input
|
|
attacker-controlled, no upstream sanitizer.
|
|
- Editor.jsx:125 feeds marked output into TipTap `setContent`. NOT a raw
|
|
sink — ProseMirror's schema normalizes/drops unknown nodes & attrs.
|
|
Marked this path as solid, not a finding.
|
|
|
|
4. SSRF — checked every outbound-fetch module:
|
|
- gitea.py: httpx client `base_url` fixed from config; owner/repo/path are
|
|
gitea API path segments, not arbitrary hosts.
|
|
- docs_sessions.py / docs_specs.py: fetch from raw/contents bases that come
|
|
from env (operator) or framework defaults; the per-spec `url` is taken
|
|
only from the operator-configured OHM_DOCS_SPECS manifest, never from
|
|
user input — user can only select a pre-registered slug.
|
|
- providers.py: provider base_url/model from env only.
|
|
- turnstile.py: siteverify URL from env or fixed constant.
|
|
- email.py: SMTP host from env.
|
|
No user-controlled fetch URL anywhere. No SSRF.
|
|
|
|
5. Path traversal — docs_sessions / docs_specs:
|
|
- api.py:242-280 validates `nnnn` against `^\d{4}$` and `filename` against
|
|
a fully-anchored `_TRANSCRIPT_FILENAME_RE` (only SESSION-...md shape, no
|
|
`/`/`..`) BEFORE any network call. docs_specs uses anchored
|
|
`^[a-z0-9-]+$` slug guard, re-checked inside fetch_spec as defense in
|
|
depth. FastAPI `{filename}` path param won't match `/` by default; even a
|
|
decoded `%2f` fails the anchored regex. No traversal.
|
|
|
|
6. Email / markdown sanitization on backend:
|
|
- email_invite.py / email_otc.py / email.py / digest.py all call
|
|
build_envelope with body_plain only. Grep confirms NO caller passes
|
|
`body_html=` anywhere — the multipart/alternative HTML branch in
|
|
email_envelope.py:134-140 is dead in practice. All mail is plain text;
|
|
user content (custom_message, display names) is plain-text interpolated.
|
|
No email HTML-injection vector at v0.24.0.
|
|
|
|
## Cut state — findings
|
|
|
|
- F1 [High] Stored XSS via unsanitized markdown→HTML. marked + raw HTML sink
|
|
(innerHTML / dangerouslySetInnerHTML), no sanitizer in pipeline or deps.
|
|
Sinks: MarkdownPreview.jsx:101-102; ProposalView.jsx:164,172. Sources:
|
|
RFC body, proposal body/use-case (any invited user), session transcripts.
|
|
Confidence: HIGH (empirically confirmed marked passthrough; sink reachable;
|
|
no upstream sanitization; backend stores raw). Invite-gating narrows the
|
|
attacker pool but stored XSS by an invited user against admins/others
|
|
(incl. admin session/cookie theft) remains. Fix: add DOMPurify and sanitize
|
|
marked output before every innerHTML/dangerouslySetInnerHTML write (single
|
|
shared helper), or render via a sanitizing markdown component; disable raw
|
|
HTML and screen javascript:/data: URIs.
|
|
|
|
- F2 [Info] Dead HTML-email branch. email_envelope.py supports a text/html
|
|
alternative but no caller supplies body_html. Not exploitable today; if
|
|
HTML email is ever added, user content must be HTML-escaped first. Track so
|
|
it's not turned on without escaping.
|
|
|
|
## What's solid
|
|
|
|
- SQL: consistent, correct parameterization across all ~16 dynamic-SQL sites;
|
|
IN-clauses use the `"?" * len` idiom; dynamic column/where fragments are
|
|
hardcoded allowlists; no ORDER BY/LIMIT/table-name injection; limits are
|
|
bounded ints.
|
|
- SSRF: zero user-controlled outbound-fetch URLs; all bases env/config-fixed.
|
|
- Path traversal: anchored-regex validation at the route layer before any
|
|
network call on both docs surfaces; defense-in-depth re-check in docs_specs.
|
|
- Editor.jsx markdown flows through TipTap/ProseMirror schema (not a raw sink).
|
|
- Email is plain-text only today.
|
|
|
|
## What the driver needs to know
|
|
|
|
- One real, shippable finding: F1 stored XSS. It is the highest-impact item
|
|
on this surface. Recommend a single sanitize helper (DOMPurify) wrapping
|
|
every marked render; covers MarkdownPreview, ProposalView, and all docs
|
|
views at once.
|
|
- Everything else on this surface is clean. SQL/SSRF/path-traversal are well
|
|
built and consistent — worth calling out as defensive strengths in the
|
|
combined audit report.
|
|
|
|
## §19.2 candidates (flotilla-core extraction)
|
|
|
|
- None from this audit — findings are rfc-app application-layer, not flotilla
|
|
CLI / deployment-shape concerns.
|