catacomb/SECURITY_AUDIT.md
Luna 4d83999edd Add statistics view, editable source_url, rewrite security audit
Statistics
- New `stats` module computes totals, top-N channels, per-year upload
  histogram, and per-week download activity from the in-memory library
  + watched/positions data.
- GET /api/stats endpoint returns the full report as JSON.
- Web UI: 📊 button in the header opens a stats modal with summary
  tiles, two CSS bar charts (weeks + years), and side-by-side top-N
  tables.
- Desktop GUI: 📊 Stats toggle in the top bar opens a window with
  the same data, rendered via egui rects (stdlib calendar math so
  there's no chrono dep).
- Bar chart helper `draw_bars` is reusable across stats sections.

Source URL (AGPL §13)
- `web.source_url` is now editable in both Settings UIs — was
  previously documented-only and required hand-editing config.toml.
- Settings POST persists it and the footer link updates immediately.
- Default config.toml ships with the Codeberg URL set.

SECURITY_AUDIT.md
- Re-audited the codebase as of commit 5999673; rewrote the file to
  document the threat model and which adversary each defense addresses.
- Resolved-from-2026-05-17 column shows what landed: session expiry,
  Secure cookie, rate-limit, body cap, CSP, X-Frame-Options,
  X-Content-Type-Options, safeUrl(), SHA-256 verify, chmod 0600 db.
- Documented accepted risks (plain-HTTP LAN, cookies.txt umask,
  job log stderr verbatim).
- Future-hardening checklist preserved for posterity.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 05:34:29 -07:00

12 KiB

Security Audit — yt-offline

Date: 2026-05-23 (re-audit after security hardening commit 5999673) Earlier audit: 2026-05-17 (commit pre-dating session auth, CSP, SHA-256 verify, etc.) Scope: Rust codebase + embedded web UI + dependencies Threat model: Self-hosted personal archiving tool. Primary deployment is single-user localhost; secondary is LAN / Tailscale access (potentially multiple devices owned by the same user); tertiary is public HTTP via reverse proxy with TLS.


Summary

Overall risk: LOW for localhost and Tailscale deployments. ⚠️ MEDIUM for plain-HTTP LAN exposure (mitigated by SameSite=Strict cookie

  • CSP; defense-in-depth, no transport encryption).

The hardening commit landed the recommendations of the prior audit plus a broader set found during code review (see commit 5999673). All findings below are now either resolved or accepted-risk-with-documentation.


Threat model

Adversary Capability In scope?
Local user on same machine Reads files owned by luna Yes — DB chmod, cookies.txt
LAN attacker on same subnet Sees HTTP traffic, can connect to bound ports Yes — bind policy, auth, CSP
Remote attacker via Tailscale tailnet Owned by same user, but still adversarial in principle Yes
Public internet attacker Reaches server through misconfigured reverse proxy Out of scope (not the intended deployment), but defenses still apply
Browser-side XSS via injected video metadata Compromised channel name / description from yt-dlp Yes — primary XSS surface
Supply-chain attack on bundled binaries Malicious yt-dlp / deno served from GitHub Yes — SHA-256 verify

The tool does not defend against an attacker with shell access to the machine, an attacker who can write to the channels directory, or compromise of upstream YouTube cookies the user has chosen to import.


Findings

Command injection — SECURE

All external process invocations use std::process::Command / tokio::process::Command with separate .arg() calls; no shell interpretation. Sites audited:

  • yt-dlp downloads: src/downloader.rs:start, start_music, repair
  • yt-dlp preview: src/web.rs:get_preview (now goes through apply_cookie_flags and respects bundled-binary setting)
  • yt-dlp bundled install: src/ytdlp_bin.rs:install_command runs bash -c with a static script whose only interpolated values are paths under ~/.local/share/yt-offline/ and constant GitHub URLs — no user input flows in.
  • ffmpeg transcoder: src/web.rs:get_transcode (kill_on_drop set; child terminates when stream is dropped).

Path interpolation in the install script uses single quotes around {dir_str}. The path is derived from $HOME via std::env::var_os. A malicious $HOME containing a single quote would break the script but isn't a real attack vector (an attacker who controls $HOME already controls the process).

SQL injection — SECURE

All queries in src/database.rs use parameterized statements (?1 / rusqlite::params!). No string concatenation in SQL. Re-verified after the settings table was added.

Path traversal — SECURE

  • /files/* and /music-files/*: served by tower-http's ServeDir, which rejects .. and refuses to follow symlinks out of the served root.
  • /api/sub-vtt/*path: canonicalises both root and target and rejects mismatched prefixes (src/web.rs:get_sub_vtt).
  • /api/transcode/:id, /api/chapters/:id, /api/metadata/:id, /api/description/:id: all resolve through library::find_video, which only returns files indexed by the scanner — direct path injection is impossible.
  • maintenance::remove_files: now canonicalises the parent + filename so missing-file cases produce accurate verdicts, and refuses any target outside the channels root (src/maintenance.rs:is_within).

XSS in embedded web UI — DEFENDED

  • All user-influenced strings (channel names, video titles, descriptions, uploader names, IDs) are escaped via esc() before HTML interpolation.
  • URLs in src= attributes go through safeUrl(), which strips javascript:, vbscript:, data:text:, and data:application: schemes (src/web.rs).
  • Content-Security-Policy middleware caps blast radius if a single esc() site is ever missed: default-src 'self'; script-src 'self' 'unsafe-inline'; ...; object-src 'none'; frame-ancestors 'none'.
  • X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy: no-referrer set globally.

Known limitation: the embedded UI uses inline <script>, so 'unsafe-inline' is required in script-src. This means a successful XSS injection could execute arbitrary inline JS, but injection itself is blocked at the input-handling layer and CSP still prevents remote script loading or framing.

Authentication & sessions — HARDENED

  • Argon2-hashed password in the settings table (never in config.toml).
  • 256-bit rand::thread_rng() (CSPRNG) session tokens, hex-encoded.
  • Mutex<HashMap<String, Instant>> of issued tokens, lazily pruned past 30-day TTL — no unbounded growth.
  • Set-Cookie includes HttpOnly, SameSite=Strict, Path=/, Max-Age=2592000. Secure is added when X-Forwarded-Proto: https is present (so reverse-proxied deployments get it; plain-HTTP LAN does not, which is correct since browsers would otherwise refuse to send the cookie).
  • All routes layered behind auth_middleware when a password is configured. /api/login and unauthenticated GET / (serves login page) are the only exceptions.
  • password_required is cached in an AtomicBool so the auth middleware doesn't hit SQLite on every static-file fetch.

Login brute-force resistance — RATE LIMITED

  • Argon2 verification (~100 ms) imposes a per-attempt cost.
  • Plus per-IP rate limiter: 5 failures within the lockout window → 60 s lockout returning HTTP 429 (src/web.rs:post_login).
  • Successful login resets the counter for that IP.
  • Lockout entries garbage-collected on each request.

Bundled-binary supply chain — VERIFIED

  • yt-dlp_linux (or _macos / .exe) is fetched over HTTPS from the official GitHub release.
  • The matching SHA2-256SUMS file is fetched from the same release and the binary's SHA-256 is compared. Mismatch aborts the install and deletes the partial binary.
  • deno is fetched over HTTPS; its SHA-256 is printed in the install log for visual inspection (deno doesn't ship a single SHA256SUMS-equivalent manifest in a stable location). A future improvement would fetch .sha256sum per-asset where available.
  • Installer sanity-checks that curl, unzip, and sha256sum are present before starting.
  • Every bundled-mode launch re-applies +x defensively via ytdlp_bin::ensure_bundled_executable so a stripped exec bit can't permanently break downloads.

Unsafe code — NONE

grep -rn "unsafe" src/ returns zero results.

Body size limits — ENFORCED

A 4 MiB DefaultBodyLimit layer is attached to every route. Legitimate payloads (cookies.txt, JSON settings) are all well under 64 KiB; anything above 4 MiB is rejected before allocation.

⚠️ Plaintext HTTP on LAN — ACCEPTED RISK

When bound to 0.0.0.0 or a LAN IP, the cookie and traffic travel unencrypted over the network. Defenses:

  • SameSite=Strict prevents cross-site form submissions from triggering authenticated requests.
  • CSP frame-ancestors 'none' prevents click-jacking.
  • The session cookie is not marked Secure because the browser would refuse to send it back on HTTP, breaking login entirely. This is a deliberate trade-off and is documented.

Mitigation if you need stronger guarantees: put a reverse proxy with TLS in front (Caddy, nginx, Traefik) and set the X-Forwarded-Proto header. The app will then issue cookies with the Secure attribute automatically.

⚠️ cookies.txt and yt-offline.db file permissions

  • yt-offline.db: now chmod 0600 at open time on Unix (src/database.rs). Contains the Argon2 password hash and resume positions. Other local users on the same machine cannot read it.
  • cookies.txt: not auto-chmodded because the user pastes / file-picks it in, and our write_cookies() uses std::fs::write which respects the user's umask. Recommendation in README: chmod 600 cookies.txt after import. (Tracking issue for future automation.)

⚠️ Job log echoes yt-dlp stderr verbatim

yt-dlp's stderr is captured into Job::log (src/downloader.rs:spawn_job). For routine failures this is fine, but auth-related errors can mention cookie file paths or impersonation profile names. In a strictly local deployment this is acceptable; if the UI is shared with someone else, review logs before sharing.

⚠️ TOCTOU on get_sub_vtt

After the path-traversal check passes, the file is opened. A symlink swap between check and read would let a local attacker substitute a different file — but the attacker would already need write access to the channels root, at which point they own the data anyway. Accepted.

⚠️ Dependency notes

  • paste v1.0.15 (transitive via egui) is unmaintained. Procedural macro, no runtime exposure. Same finding as 2026-05-17 audit; nothing changed upstream. Re-check periodically.
  • cargo audit was not re-run for this audit; recommended before publishing release artifacts.

Changes since the 2026-05-17 audit

Item Status
Bind defaults to 127.0.0.1 already in 2026-05-17
Content-Security-Policy header added (security_headers middleware)
Optional HTTP password auth added (Argon2 + sessions + rate limit)
Filter yt-dlp stderr ⚠️ still echoed (accepted)
chmod 600 on cookies.txt ⚠️ documented; not automated
chmod 600 on yt-offline.db added (Database::open)
Session token TTL + GC added
Secure cookie flag for HTTPS added (gated on X-Forwarded-Proto)
Login rate limiting added (5 / 60 s per IP)
Body size cap added (4 MiB)
Bundled binary verification added (SHA-256 + sanity checks)
safeUrl() for src= attributes added
Path traversal in maintenance::remove_files hardened (parent-canonicalize fallback)

Recommendations for future hardening

These are nice-to-haves; none are blocking the current deployment.

  1. Auto-chmod 600 cookies.txt on write. Reduces user error.
  2. Filter --cookies paths from job log. Trivial regex strip before buffering into Job::log.
  3. Optional data: URL scheme allow-list for <img> only. Useful if we ever start showing yt-dlp-supplied thumbnails as inline data URLs.
  4. cargo audit in CI. Once cross-compile CI exists.
  5. Periodic deno SHA-256 pinning. Currently we print the hash; a future enhancement could check it against a small in-repo manifest updated by a maintainer.
  6. Account lockout escalation. Current scheme resets fully after 60 s. Consider exponential back-off (60 s → 5 min → 15 min) for repeated abuse from the same IP across longer windows.
  7. TLS terminator in-process. Optional rustls integration so the app can serve HTTPS without a reverse proxy. Currently considered out of scope.

Verification commands

# Build + tests pass
cargo build --release
cargo test

# No unsafe blocks
grep -rn 'unsafe' src/

# Dependencies up to date
cargo update --dry-run
cargo audit          # requires the cargo-audit binary

Auditor: Claude Code Audit method: Manual code review of every src/*.rs plus the embedded HTML/JS UI; targeted greps for command/SQL/path patterns; verification of hardening claims against current source. Status: Complete.