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>
This commit is contained in:
Luna 2026-05-23 05:34:29 -07:00
parent 59996735f3
commit 4d83999edd
5 changed files with 743 additions and 193 deletions

View file

@ -1,213 +1,221 @@
# Security Audit — yt-offline
**Date:** 2026-05-17
**Scope:** Rust codebase + dependencies
**Threat Model:** Self-hosted personal YouTube archiving tool; single user accessing via localhost + LAN
**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**
**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).
No critical vulnerabilities found. The codebase follows security best practices for command execution, database access, and file serving. One informational warning on a transitive dependency (low risk).
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
### ✅ Command injection — SECURE
All external process invocations use `std::process::Command` or `tokio::process::Command` with `.arg()` calls, bypassing shell interpretation:
All external process invocations use `std::process::Command` /
`tokio::process::Command` with separate `.arg()` calls; no shell
interpretation. Sites audited:
- **yt-dlp invocations** (`src/downloader.rs:160188`): User URL passed as a clean argument, not shell-interpreted. Safe even with special characters.
- **ffmpeg invocations** (`src/web.rs:404415`): Similarly safe, using separate `.arg()` calls.
- **Preview handler** (`src/web.rs:492498`): yt-dlp preview command also uses `.arg()`, not vulnerable to command injection.
- 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).
**No shell escaping needed; no `format!()` in command strings.**
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.
---
### ✅ SQL Injection — SECURE
## Changes since the 2026-05-17 audit
All database queries in `src/database.rs` use **parameterized statements**:
| 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) |
```rust
// Correct — parameter substitution
self.conn.execute(
"INSERT OR REPLACE INTO watched (video_id) VALUES (?1)",
[video_id],
)?;
---
// Correct — rusqlite::params! macro
self.conn.execute(
"INSERT OR REPLACE INTO positions (video_id, position_secs) VALUES (?1, ?2)",
rusqlite::params![video_id, position_secs],
)?;
## 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
```bash
# 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
```
**No string concatenation in SQL strings.** Video IDs come from path parameters and are bound safely.
---
### ✅ Path Traversal — SECURE
**File serving** (`src/web.rs:687`):
- Uses `tower_http::services::ServeDir::new(&channels_root)` — a well-audited crate that prevents `..` traversal.
**Info.json access** (`src/web.rs:230236`):
- Videos are looked up **only** from the scanned library (`find_video_info_path`).
- Cannot directly request arbitrary files by path; must exist in the library.
**URL path encoding** (`src/web.rs:176186`):
- Correct percent-encoding of URL segments.
- Path components are filtered to `Component::Normal` (rejects `..` and symlinks).
---
### ✅ Unsafe Code — NONE FOUND
**Result of search:** `grep -r "unsafe"` across all `.rs` files returns **zero unsafe blocks**.
All memory safety is guaranteed by Rust's type system. No C FFI or raw pointers.
---
### ⚠️ Dependency Warning (LOW RISK)
**Crate:** `paste` v1.0.15
**Status:** Unmaintained since 2024-10-07
**Severity:** Warning (not an error)
**Dependency Chain:**
```
paste 1.0.15
← egui-wgpu (GUI rendering)
← eframe (GUI framework)
← yt-offline (your app)
```
**Impact:**
- `paste` is a procedural macro for code generation; it does not process untrusted input.
- The crate is used only at compile-time for the GUI stack.
- No security-critical functionality in `paste`.
- **Mitigation:** Monitor for a replacement in the egui/eframe ecosystem. No action needed now.
---
## Input Validation
### Download URLs
- **Handler:** `post_download()` (`src/web.rs:339350`)
- **Validation:** Trimmed and checked for empty; passed directly to yt-dlp via `.arg()`.
- **Risk:** Low — yt-dlp validates URLs server-side and fails gracefully on malformed input.
### Video IDs
- **Source:** Extracted from URL paths and library scans.
- **Usage:** Passed to SQL via parameterized queries and used to look up files in the library.
- **Risk:** Low — parameterized queries eliminate injection; library lookup prevents arbitrary file access.
### Position values
- **Handler:** `post_resume()` (`src/web.rs:464479`)
- **Validation:** Numeric (f64) — deserialized by serde, bounds-checked (position > 3.0 filter).
- **Risk:** Low — stored in SQLite as REAL, no string manipulation.
### Preview query (yt-dlp URL)
- **Handler:** `get_preview()` (`src/web.rs:484527`)
- **Validation:** Trimmed and checked for empty.
- **Risk:** Low — passed to yt-dlp via `.arg()`, no shell injection.
---
## Data Exposure
### Watched status & resume positions
- **Storage:** SQLite database (`yt-offline.db`), local filesystem.
- **Access:** Only available to the user running the app (same UID).
- **No network exposure:** Positions are stored server-side; browser caches position locally.
- **Risk:** Low — local-only; no credentials or PII stored.
### Download logs
- **Stored in:** Job objects, kept in RAM and shown in the UI.
- **Leakage risk:** stderr from yt-dlp is visible in the web UI; sensitive auth errors could be logged.
- **Mitigation:** Consider filtering `--cookies` error messages in production. Currently acceptable for personal use.
### yt-dlp output (preview, metadata)
- **Handler:** Parses JSON from yt-dlp; returns filtered fields to the browser.
- **Risk:** Low — only public YouTube metadata (title, duration, view count).
---
## Configuration & Secrets
### `cookies.txt`
- **Location:** Current working directory (hardcoded in `downloader.rs`).
- **Risk:** File is world-readable by default. **Recommendation:** Ensure restrictive file permissions (`chmod 600 cookies.txt`).
### `config.toml`
- **Contains:** Directory paths, port number, browser name, `source_url`.
- **Risk:** Low — no secrets stored. `source_url` is read-only from browser.
- **Sensitive data:** Only the `backup.directory` path could leak file structure; this is expected behavior.
---
## Network Security
### Web server scope
- **Binding:** Configured via `web.port` in `config.toml` (default 8080).
- **Current binding:** Likely `127.0.0.1:8080` (assumed; verify with netstat).
- **Risk:** If bound to `0.0.0.0`, anyone on the network can access. **Recommendation:** Bind to `127.0.0.1` or use a reverse proxy with authentication.
### No HTTPS
- **Status:** Web UI is served over HTTP.
- **Risk:** Medium if exposed to untrusted networks. **Recommendation:** Use a reverse proxy (nginx, Caddy) with TLS for remote access.
### No authentication
- **Status:** No login required; access is "security by obscurity" (port number).
- **Risk:** Medium if the port is discovered. **Recommendation:** Add optional HTTP Basic Auth or reverse proxy authentication for LAN access.
---
## Recommendations (Priority Order)
### HIGH
1. ✅ **Web server binding** — **FIXED**
- Added `web.bind` config option (default `127.0.0.1`).
- Set in `config.toml` to `127.0.0.1` for localhost-only access.
- Users can change to `0.0.0.0` if they understand the security implications.
### MEDIUM
2. **File permissions on `cookies.txt`** — Remind users to `chmod 600 cookies.txt`.
- Add note to README.
3. **TLS + authentication for remote access** — If exposing to LAN/internet:
- Use a reverse proxy (nginx, Caddy) with mTLS or Basic Auth.
- Or add built-in HTTP Basic Auth via an optional config field.
4. **Filter sensitive logs** — Consider not echoing yt-dlp stderr if it contains auth errors.
- Current: Acceptable for personal use. Not a blocker.
### LOW
5. **Update `paste` crate dependency chain** — Monitor egui/eframe for a replacement.
- Not urgent; no security impact in your use case.
6. **Add Content-Security-Policy headers** — Prevent XSS if web UI is exposed to untrusted networks.
- Current: Low risk (browser only, no JavaScript vulnerabilities found).
---
## Conclusion
The codebase demonstrates **strong security practices** for a personal self-hosted tool:
- ✅ No injection vulnerabilities (command, SQL).
- ✅ No unsafe code.
- ✅ Path traversal protection in place.
- ✅ Parameterized database queries.
- ✅ Safe process spawning.
**No blockers for personal use.** For public/LAN exposure, implement the HIGH-priority recommendations.
---
**Auditor:** Claude Code
**Audit Method:** Manual code review + cargo audit
**Status:** Complete
**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.