fix(security): CORS allowlist, URL scheme guard, state perms, ZIP slip, mirror docs
Phase 3 security audit follow-up. Medium and low-severity hardenings plus a deferred-work plan for the cross-repo stream-token rollout. Stream server CORS: replace the wildcard Access-Control-Allow-Origin with an allowlist that echoes back only torrentclaw.com, app.torrentclaw.com, the local Next dev port (3030 — matches the web repo package.json) and any extras the operator adds via the new downloads.cors_extra_origins TOML key. A Vary: Origin header is now emitted whenever the request carries an Origin header so an intermediate cache cannot serve a stale ACAO to a different origin. URL scheme guard: openBrowser and OpenPlayer refuse any URL that is not http(s). Combined with passing the URL after "--" wherever the launched helper supports it (open, mpv, vlc, cvlc), this stops a leading "-" from being parsed as a switch by the spawned process. State file permissions: WriteState now writes 0o600 so the agent ID, PID and counters cannot be enumerated by another local user on a shared host. Matches the existing config file mode. ZIP slip defense-in-depth: extractZip extracts the safety check into safeZipPath, which canonicalises the entry name (normalising backslashes to "/"), rejects "..", "../" prefix and "/../" interior components, and verifies the final destination stays inside destDir before opening any file. Mirror fallback: documented the design for multi-provider mirrors.json hosting in the comment block on DefaultStaticFallbackURLs and added a follow-up note about signing it with the same ed25519 release key. The list is kept at one provider until the second host is provisioned and added to torrentclaw-web's STATIC_FALLBACKS. Deferred work: a new plan document Docs/plans/security-stream-token.md covers the per-task stream token (Phase 2.2 of the original audit) which requires coordinated web + CLI work and ships separately.
This commit is contained in:
parent
433e375def
commit
060a3e48db
13 changed files with 462 additions and 48 deletions
131
Docs/plans/security-stream-token.md
Normal file
131
Docs/plans/security-stream-token.md
Normal file
|
|
@ -0,0 +1,131 @@
|
|||
# Phase 2.2 — Per-task stream token (deferred)
|
||||
|
||||
Status: deferred. Requires coordinated change in the web app
|
||||
(`torrentclaw-web`) and the CLI daemon. Pulled out of the Phase 2
|
||||
security pass because the CLI-only fixes (UPnP opt-in, SSE caps,
|
||||
signed self-update) ship without web-side work; the stream-token
|
||||
work cannot.
|
||||
|
||||
## Problem
|
||||
|
||||
`/stream`, `/playlist.m3u` and `/hls/<sessionID>/...` on the daemon
|
||||
HTTP server have no authentication. Today, anyone who can reach the
|
||||
listener and guesses (or learns) the `taskID` (for `/stream`) or
|
||||
`sessionID` (for `/hls`) can fetch the active file.
|
||||
|
||||
Mitigations already in place after Phase 1+2:
|
||||
|
||||
- `sessionID` is restricted to a safe regex and is a server-issued
|
||||
UUID v4 (122-bit entropy, not enumerable in practice).
|
||||
- `/health` no longer leaks the active filename, taskID prefix or
|
||||
client IP to remote callers (loopback diagnostics preserved).
|
||||
- UPnP is opt-in, so by default the daemon is not exposed to the
|
||||
public internet.
|
||||
- The web client probes `/health` to pick LAN vs Tailscale.
|
||||
|
||||
Residual risk:
|
||||
|
||||
- On a shared LAN (open Wi-Fi, office network, dorm) any device can
|
||||
reach the listener and brute-force `?id=<taskID>` against
|
||||
`/stream`. taskIDs are also UUIDs, so this is high entropy, but
|
||||
the URL may leak through browser history, sharing, screen capture
|
||||
or a passive logger and there is no second factor.
|
||||
- A user who explicitly opts into UPnP exposes the same surface to
|
||||
the entire internet.
|
||||
|
||||
A per-task secret carried in the URL closes this without breaking
|
||||
the `<video src>` flow (the browser cannot attach `Authorization`
|
||||
headers to media elements, but it can append a query parameter).
|
||||
|
||||
## Design
|
||||
|
||||
Both ends agree on a per-task secret token. The web generates it
|
||||
when the user requests streaming; the daemon receives the
|
||||
`(taskID, token)` pair and validates the token on every `/stream`
|
||||
and `/hls/...` request.
|
||||
|
||||
### Web side (`torrentclaw-web`)
|
||||
|
||||
When the user clicks "Stream":
|
||||
|
||||
1. Generate `streamToken = crypto.randomBytes(32).toString("hex")`
|
||||
server-side (NOT browser, so it never lives in client storage
|
||||
longer than the page lifetime).
|
||||
2. Persist `(taskID, streamToken, expiresAt)` in `download_task`
|
||||
(new columns or a sibling table). Token expires e.g. 6 h after
|
||||
issue or on explicit revoke.
|
||||
3. Push the token to the daemon over the existing heartbeat / sync
|
||||
channel that already carries `streamRequested`. Add a
|
||||
`streamToken` field next to it. The daemon trusts that channel
|
||||
(it is authenticated agent ↔ origin).
|
||||
4. Include the token in the stream URLs the API returns to the
|
||||
browser:
|
||||
`http://<host>:<port>/stream?id=<taskID>&t=<streamToken>` and
|
||||
the `/hls/<sessionID>` URLs gain `?t=<streamToken>` too.
|
||||
|
||||
Files that will need to change:
|
||||
|
||||
- `src/lib/services/agent.ts` — extend the stream-request payload
|
||||
with `streamToken`.
|
||||
- `src/lib/db/schema.ts` — column / table for the token.
|
||||
- `src/lib/services/stream-resolve.ts` — append `&t=` to the URLs
|
||||
it builds.
|
||||
- `src/lib/stream-probe.ts` — keep probing `/health` (no token),
|
||||
then append `&t=` to the winning stream URL before returning.
|
||||
- `src/middleware.ts` — no CORS change required (browser still hits
|
||||
daemon directly).
|
||||
|
||||
### CLI side
|
||||
|
||||
- `internal/agent/types.go` / `internal/agent/sync.go` — accept and
|
||||
store `streamToken` next to `streamRequested`.
|
||||
- `internal/agent/daemon.go` — when the heartbeat reports a new
|
||||
active stream task, push the token into the stream server via a
|
||||
setter: `streamSrv.SetTaskToken(taskID, token)`.
|
||||
- `internal/engine/stream_server.go`:
|
||||
- New field `tokens map[string]string` guarded by mutex.
|
||||
- `SetTaskToken(taskID, token)` and `ClearTaskToken(taskID)`.
|
||||
- `handler` (`/stream`) extracts `?id=` and `?t=`, checks the
|
||||
token with `subtle.ConstantTimeCompare`; 404 on mismatch.
|
||||
- `hlsHandler` (`/hls/<sessionID>/...`) needs an HLS-session
|
||||
→ token mapping, since the path carries `sessionID` not
|
||||
`taskID`. Store the token on the `HLSSession` at start and
|
||||
validate per request.
|
||||
|
||||
### Backwards compatibility
|
||||
|
||||
- The daemon must accept token-less requests for one minor version
|
||||
so a newer daemon can still serve an older web (and vice-versa).
|
||||
Gate the check on a config flag (`require_stream_token`,
|
||||
default false in the first release, default true in the next).
|
||||
- The `<video src>` form supports query parameters, so the only
|
||||
user-visible change is the URL string.
|
||||
|
||||
## Open questions to resolve before implementing
|
||||
|
||||
1. Token TTL. 6 h gives plenty of room for a movie + pause +
|
||||
resume; longer means the post-leak window is wider.
|
||||
2. Where to store the token in `download_task` — same row, or a
|
||||
sibling `download_stream_token` table that we can rotate
|
||||
without writing to the task row.
|
||||
3. Should `/playlist.m3u` (VLC) embed the token directly, or use
|
||||
a one-shot redeem URL? VLC URL ends up in history.
|
||||
4. Token reuse across HLS reconnects — yes, scoped to the
|
||||
`HLSSession`, invalidated on `Close()`.
|
||||
5. Do we want a daemon flag `--require-stream-token` independent
|
||||
of config, for users to flip on quickly without editing TOML?
|
||||
|
||||
## Effort estimate
|
||||
|
||||
- CLI: ~3 h
|
||||
- Web: ~3 h
|
||||
- Migration + rollout (config flag flip): 1 release cycle of soak.
|
||||
|
||||
## Why not now
|
||||
|
||||
- Cross-repo coordination raises commit blast radius beyond what
|
||||
the Phase 2 security pass should carry.
|
||||
- Web work needs DB migration + UI surfaces (the "stream link
|
||||
expired" path).
|
||||
- Phase 2 hardenings ship value today without it; this is the
|
||||
defense-in-depth layer on top.
|
||||
|
|
@ -37,7 +37,17 @@ type MirrorsResponse struct {
|
|||
// Hard-coded here (not loaded from config) because the whole point is to
|
||||
// have something to consult when config-driven URLs all fail.
|
||||
//
|
||||
// Keep in sync with src/lib/mirrors-config.ts → STATIC_FALLBACKS on the web.
|
||||
// Today there is one provider (GitHub Pages). The slice is intentionally
|
||||
// shaped to take more — a second independent host (Cloudflare Pages,
|
||||
// IPFS-Fleek, etc.) should be added as soon as it is provisioned. Keep
|
||||
// any addition in sync with `STATIC_FALLBACKS` in
|
||||
// `torrentclaw-web/src/lib/mirrors-config.ts` and `Docs/plans/security-stream-token.md`.
|
||||
//
|
||||
// Future hardening: sign mirrors.json with the same ed25519 release key
|
||||
// (or a sibling) so a hijack of any single static host cannot serve a
|
||||
// malicious mirror list. Today the only signal is "agreement between
|
||||
// independent providers" via cross-checking, which we leave to the
|
||||
// operator.
|
||||
var DefaultStaticFallbackURLs = []string{
|
||||
"https://torrentclaw.github.io/mirrors/mirrors.json",
|
||||
}
|
||||
|
|
|
|||
|
|
@ -45,9 +45,13 @@ func WriteState(state *DaemonState) {
|
|||
return
|
||||
}
|
||||
|
||||
// Write to temp file then rename for atomicity
|
||||
// Write to temp file then rename for atomicity. 0o600 keeps the file
|
||||
// readable only by the owning user — the state contains agentID, PID
|
||||
// and counters which are useful to a co-tenant on a shared host for
|
||||
// fingerprinting the daemon, and we already use 0o600 for the config
|
||||
// file. No need for cross-user readability here.
|
||||
tmp := path + ".tmp"
|
||||
if err := os.WriteFile(tmp, data, 0o644); err != nil {
|
||||
if err := os.WriteFile(tmp, data, 0o600); err != nil {
|
||||
return
|
||||
}
|
||||
os.Rename(tmp, path)
|
||||
|
|
|
|||
|
|
@ -241,6 +241,7 @@ func runDaemonStart() error {
|
|||
// Create persistent stream server
|
||||
streamSrv := engine.NewStreamServer(cfg.Download.StreamPort)
|
||||
streamSrv.SetUPnPEnabled(cfg.Download.EnableUPnP)
|
||||
streamSrv.SetCORSAllowedOrigins(cfg.Download.CORSExtraOrigins)
|
||||
// Reap HLS tmpdirs left over from a previous daemon run before we start
|
||||
// accepting new sessions. The in-memory registry doesn't survive a
|
||||
// restart, so without this disk usage grows unbounded across restarts.
|
||||
|
|
|
|||
|
|
@ -9,12 +9,21 @@ import (
|
|||
)
|
||||
|
||||
// openBrowser opens a URL in the default browser.
|
||||
//
|
||||
// The URL is restricted to http(s) so that a hostile caller cannot trick
|
||||
// xdg-open/open into interpreting it as a flag (a leading "-" would otherwise
|
||||
// match a switch on every helper we shell out to). Where the helper supports
|
||||
// it we also append "--" to terminate switch parsing as belt-and-braces.
|
||||
func openBrowser(url string) {
|
||||
if !isSafeBrowserURL(url) {
|
||||
return
|
||||
}
|
||||
var c *exec.Cmd
|
||||
switch runtime.GOOS {
|
||||
case "darwin":
|
||||
c = exec.Command("open", url)
|
||||
c = exec.Command("open", "--", url)
|
||||
case "windows":
|
||||
// rundll32 does not parse switches from positional args.
|
||||
c = exec.Command("rundll32", "url.dll,FileProtocolHandler", url)
|
||||
default: // linux, freebsd
|
||||
c = exec.Command("xdg-open", url)
|
||||
|
|
@ -22,6 +31,12 @@ func openBrowser(url string) {
|
|||
_ = c.Start() // fire and forget; best-effort
|
||||
}
|
||||
|
||||
// isSafeBrowserURL accepts only http(s) URLs. Other schemes (file://, javascript:,
|
||||
// data:, ...) and flag-shaped strings ("--help") are rejected.
|
||||
func isSafeBrowserURL(url string) bool {
|
||||
return strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://")
|
||||
}
|
||||
|
||||
// defaultDownloadDir returns a sensible default download directory.
|
||||
func defaultDownloadDir() string {
|
||||
home, _ := os.UserHomeDir()
|
||||
|
|
|
|||
|
|
@ -31,6 +31,32 @@ func TestExpandHome(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestIsSafeBrowserURL(t *testing.T) {
|
||||
good := []string{
|
||||
"http://localhost:3000",
|
||||
"https://torrentclaw.com/some/path?q=1",
|
||||
}
|
||||
bad := []string{
|
||||
"--help",
|
||||
"-version",
|
||||
"file:///etc/passwd",
|
||||
"javascript:alert(1)",
|
||||
"data:text/html,foo",
|
||||
"ftp://example.com",
|
||||
"",
|
||||
}
|
||||
for _, u := range good {
|
||||
if !isSafeBrowserURL(u) {
|
||||
t.Errorf("isSafeBrowserURL(%q) = false, want true", u)
|
||||
}
|
||||
}
|
||||
for _, u := range bad {
|
||||
if isSafeBrowserURL(u) {
|
||||
t.Errorf("isSafeBrowserURL(%q) = true, want false", u)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultDownloadDir(t *testing.T) {
|
||||
dir := defaultDownloadDir()
|
||||
if dir == "" {
|
||||
|
|
|
|||
|
|
@ -50,6 +50,7 @@ type DownloadConfig struct {
|
|||
ListenPort int `toml:"listen_port"` // fixed port for incoming peer connections (default: 42069, 0 = random)
|
||||
StreamPort int `toml:"stream_port"` // fixed port for streaming HTTP server (default: 11818)
|
||||
EnableUPnP bool `toml:"enable_upnp"` // map StreamPort to the WAN via UPnP/NAT-PMP (default: false; opt-in because it exposes the unauthenticated /stream + /hls endpoints to the public internet)
|
||||
CORSExtraOrigins []string `toml:"cors_extra_origins"` // extra browser origins added on top of the baked-in allowlist (torrentclaw.com, app.torrentclaw.com, localhost:3030)
|
||||
WebRTC WebRTCConfig `toml:"webrtc"`
|
||||
Transcode TranscodeConfig `toml:"transcode"`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,14 +4,23 @@ import (
|
|||
"fmt"
|
||||
"os/exec"
|
||||
"runtime"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// OpenPlayer attempts to open a media player with the given stream URL.
|
||||
// Returns the player name and the running command.
|
||||
// If override is set, it uses that command directly.
|
||||
//
|
||||
// The URL is required to be http(s) so a hostile-looking value (e.g. starting
|
||||
// with `--`) is not interpreted as a switch by mpv/vlc/xdg-open/open. The
|
||||
// `--` separator is also appended before the URL where the helper supports
|
||||
// it.
|
||||
func OpenPlayer(url, override string) (string, *exec.Cmd, error) {
|
||||
if !isSafePlayerURL(url) {
|
||||
return "", nil, fmt.Errorf("refusing to open non-http(s) URL")
|
||||
}
|
||||
if override != "" {
|
||||
cmd := exec.Command(override, url)
|
||||
cmd := exec.Command(override, "--", url)
|
||||
if err := cmd.Start(); err != nil {
|
||||
return override, nil, fmt.Errorf("start %s: %w", override, err)
|
||||
}
|
||||
|
|
@ -20,7 +29,7 @@ func OpenPlayer(url, override string) (string, *exec.Cmd, error) {
|
|||
|
||||
// Try mpv first (best streaming support)
|
||||
if path, err := exec.LookPath("mpv"); err == nil {
|
||||
cmd := exec.Command(path, "--no-terminal", url)
|
||||
cmd := exec.Command(path, "--no-terminal", "--", url)
|
||||
if err := cmd.Start(); err == nil {
|
||||
return "mpv", cmd, nil
|
||||
}
|
||||
|
|
@ -28,7 +37,7 @@ func OpenPlayer(url, override string) (string, *exec.Cmd, error) {
|
|||
|
||||
// Try VLC
|
||||
if path, err := exec.LookPath("vlc"); err == nil {
|
||||
cmd := exec.Command(path, url)
|
||||
cmd := exec.Command(path, "--", url)
|
||||
if err := cmd.Start(); err == nil {
|
||||
return "vlc", cmd, nil
|
||||
}
|
||||
|
|
@ -36,7 +45,7 @@ func OpenPlayer(url, override string) (string, *exec.Cmd, error) {
|
|||
|
||||
// Try cvlc (VLC headless)
|
||||
if path, err := exec.LookPath("cvlc"); err == nil {
|
||||
cmd := exec.Command(path, url)
|
||||
cmd := exec.Command(path, "--", url)
|
||||
if err := cmd.Start(); err == nil {
|
||||
return "vlc (headless)", cmd, nil
|
||||
}
|
||||
|
|
@ -51,6 +60,9 @@ func OpenPlayer(url, override string) (string, *exec.Cmd, error) {
|
|||
}
|
||||
|
||||
func openBrowser(url string) (string, *exec.Cmd, error) {
|
||||
if !isSafePlayerURL(url) {
|
||||
return "", nil, fmt.Errorf("refusing to open non-http(s) URL")
|
||||
}
|
||||
switch runtime.GOOS {
|
||||
case "linux":
|
||||
if path, err := exec.LookPath("xdg-open"); err == nil {
|
||||
|
|
@ -60,7 +72,7 @@ func openBrowser(url string) (string, *exec.Cmd, error) {
|
|||
}
|
||||
}
|
||||
case "darwin":
|
||||
cmd := exec.Command("/usr/bin/open", url)
|
||||
cmd := exec.Command("/usr/bin/open", "--", url)
|
||||
if err := cmd.Start(); err == nil {
|
||||
return "browser", cmd, nil
|
||||
}
|
||||
|
|
@ -72,3 +84,9 @@ func openBrowser(url string) (string, *exec.Cmd, error) {
|
|||
}
|
||||
return "", nil, fmt.Errorf("no browser opener found")
|
||||
}
|
||||
|
||||
// isSafePlayerURL guards the helpers above against URLs that could be
|
||||
// interpreted as command-line switches by the launched player.
|
||||
func isSafePlayerURL(url string) bool {
|
||||
return strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,6 +56,12 @@ type StreamServer struct {
|
|||
// would let any scanner enumerate active downloads. LAN and Tailscale
|
||||
// access keep working without UPnP.
|
||||
enableUPnP bool
|
||||
// corsExtraOrigins are operator-configured origins added to the default
|
||||
// allowlist defined in validate.go. Set before Listen().
|
||||
corsExtraOrigins []string
|
||||
// corsAllowlist is computed at Listen() time and treated as read-only
|
||||
// thereafter so per-request reads need no locking.
|
||||
corsAllowlist map[string]struct{}
|
||||
|
||||
hls *HLSSessionRegistry // HLS sessions served on /hls/<id>/...
|
||||
|
||||
|
|
@ -86,12 +92,57 @@ func (ss *StreamServer) SetUPnPEnabled(enabled bool) {
|
|||
ss.enableUPnP = enabled
|
||||
}
|
||||
|
||||
// SetCORSAllowedOrigins replaces the operator-supplied extra origins. The
|
||||
// default allowlist (torrentclaw.com / app.torrentclaw.com / localhost dev
|
||||
// ports) is always merged in. Call before Listen().
|
||||
func (ss *StreamServer) SetCORSAllowedOrigins(origins []string) {
|
||||
ss.corsExtraOrigins = origins
|
||||
}
|
||||
|
||||
// writeCORSHeaders writes the per-origin CORS response headers when the
|
||||
// request carries an Origin header that matches the allowlist. Returns true
|
||||
// if the handler must short-circuit (preflight OPTIONS). Media-tag requests
|
||||
// (no Origin header) bypass this entirely.
|
||||
//
|
||||
// `Vary: Origin` is emitted whenever an Origin header is present (matched
|
||||
// or not) so any intermediate cache keys the response per-origin and a
|
||||
// later request with a different origin cannot be served a stale ACAO.
|
||||
func (ss *StreamServer) writeCORSHeaders(w http.ResponseWriter, r *http.Request, expose string) (preflight bool) {
|
||||
origin := r.Header.Get("Origin")
|
||||
if origin == "" {
|
||||
return false
|
||||
}
|
||||
w.Header().Add("Vary", "Origin")
|
||||
if _, ok := ss.corsAllowlist[origin]; !ok {
|
||||
// Unknown origin — do not emit CORS headers so the browser blocks
|
||||
// the response. Still return without short-circuiting so a non-CORS
|
||||
// caller (e.g. curl) keeps working.
|
||||
return false
|
||||
}
|
||||
w.Header().Set("Access-Control-Allow-Origin", origin)
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Range")
|
||||
if expose != "" {
|
||||
w.Header().Set("Access-Control-Expose-Headers", expose)
|
||||
}
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// HLS returns the HLS session registry for this server. Daemon code uses it
|
||||
// to register a session when the backend asks for HLS playback.
|
||||
func (ss *StreamServer) HLS() *HLSSessionRegistry { return ss.hls }
|
||||
|
||||
// Listen starts the HTTP server on the configured port. Call once at daemon startup.
|
||||
func (ss *StreamServer) Listen(ctx context.Context) error {
|
||||
// Freeze the CORS allowlist before the first request can land. After
|
||||
// this point the map is treated as read-only so handlers can probe it
|
||||
// without locking.
|
||||
ss.corsAllowlist = buildCORSAllowlist(ss.corsExtraOrigins)
|
||||
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/stream", ss.handler)
|
||||
mux.HandleFunc("/health", ss.healthHandler)
|
||||
|
|
@ -306,16 +357,8 @@ func (ss *StreamServer) HLSURLsJSON(sessionID string) string {
|
|||
func (ss *StreamServer) hlsHandler(w http.ResponseWriter, r *http.Request) {
|
||||
ss.lastActivity.Store(time.Now().UnixNano())
|
||||
|
||||
// CORS for app.torrentclaw.com → 127.0.0.1/Tailscale daemon.
|
||||
if origin := r.Header.Get("Origin"); origin != "" {
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Range")
|
||||
w.Header().Set("Access-Control-Expose-Headers", "Content-Length, Content-Range, Accept-Ranges")
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
if ss.writeCORSHeaders(w, r, "Content-Length, Content-Range, Accept-Ranges") {
|
||||
return
|
||||
}
|
||||
|
||||
rest := strings.TrimPrefix(r.URL.Path, "/hls/")
|
||||
|
|
@ -414,6 +457,9 @@ func (ss *StreamServer) serveSubtitlePlaylist(w http.ResponseWriter, r *http.Req
|
|||
//
|
||||
// curl http://<tailscale-ip>:<port>/health
|
||||
func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) {
|
||||
if ss.writeCORSHeaders(w, r, "") {
|
||||
return
|
||||
}
|
||||
ss.mu.RLock()
|
||||
provider := ss.provider
|
||||
taskID := ss.taskID
|
||||
|
|
@ -470,15 +516,8 @@ func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) {
|
|||
// VLC fetches this playlist and applies the EXTVLCOPT directives automatically,
|
||||
// enabling automatic audio/subtitle track selection on all VLC platforms (desktop + mobile).
|
||||
func (ss *StreamServer) playlistHandler(w http.ResponseWriter, r *http.Request) {
|
||||
// CORS — handle preflight before doing any work (consistent with handler)
|
||||
if origin := r.Header.Get("Origin"); origin != "" {
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Range")
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
if ss.writeCORSHeaders(w, r, "") {
|
||||
return
|
||||
}
|
||||
|
||||
q := r.URL.Query()
|
||||
|
|
@ -548,17 +587,8 @@ func (ss *StreamServer) handler(w http.ResponseWriter, r *http.Request) {
|
|||
return
|
||||
}
|
||||
|
||||
// CORS headers — only when browser sends Origin (HTTPS site → localhost)
|
||||
if origin := r.Header.Get("Origin"); origin != "" {
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Range")
|
||||
w.Header().Set("Access-Control-Expose-Headers", "Content-Length, Content-Range, Accept-Ranges")
|
||||
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
if ss.writeCORSHeaders(w, r, "Content-Length, Content-Range, Accept-Ranges") {
|
||||
return
|
||||
}
|
||||
|
||||
rawReader := provider.NewFileReader(r.Context())
|
||||
|
|
|
|||
|
|
@ -429,6 +429,71 @@ func TestStreamServer_Health_NonLoopback_NoLeak(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestStreamServer_CORS_Allowlist verifica que sólo los origenes en la
|
||||
// allowlist reciben Access-Control-Allow-Origin y que ningún otro origen
|
||||
// es eco-reflejado.
|
||||
func TestStreamServer_CORS_Allowlist(t *testing.T) {
|
||||
srv := NewStreamServer(0)
|
||||
ctx := context.Background()
|
||||
if err := srv.Listen(ctx); err != nil {
|
||||
t.Fatalf("Listen: %v", err)
|
||||
}
|
||||
defer srv.Shutdown(ctx)
|
||||
|
||||
cases := []struct {
|
||||
origin string
|
||||
wantAllow bool
|
||||
}{
|
||||
{"https://app.torrentclaw.com", true},
|
||||
{"https://torrentclaw.com", true},
|
||||
{"http://localhost:3030", true},
|
||||
{"http://127.0.0.1:3030", true},
|
||||
{"https://evil.example", false},
|
||||
{"null", false},
|
||||
{"", false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.origin, func(t *testing.T) {
|
||||
rr := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodOptions, "/health", nil)
|
||||
if tc.origin != "" {
|
||||
req.Header.Set("Origin", tc.origin)
|
||||
}
|
||||
srv.healthHandler(rr, req)
|
||||
got := rr.Header().Get("Access-Control-Allow-Origin")
|
||||
if tc.wantAllow {
|
||||
if got != tc.origin {
|
||||
t.Errorf("origin %q: ACAO = %q, want %q", tc.origin, got, tc.origin)
|
||||
}
|
||||
} else if got != "" {
|
||||
t.Errorf("origin %q: ACAO leaked as %q, expected empty", tc.origin, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestStreamServer_CORS_ExtraOrigin verifica que SetCORSAllowedOrigins añade
|
||||
// origins al baseline sin removerlos.
|
||||
func TestStreamServer_CORS_ExtraOrigin(t *testing.T) {
|
||||
srv := NewStreamServer(0)
|
||||
srv.SetCORSAllowedOrigins([]string{"https://custom.example"})
|
||||
ctx := context.Background()
|
||||
if err := srv.Listen(ctx); err != nil {
|
||||
t.Fatalf("Listen: %v", err)
|
||||
}
|
||||
defer srv.Shutdown(ctx)
|
||||
|
||||
for _, origin := range []string{"https://custom.example", "https://torrentclaw.com"} {
|
||||
rr := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/health", nil)
|
||||
req.Header.Set("Origin", origin)
|
||||
srv.healthHandler(rr, req)
|
||||
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != origin {
|
||||
t.Errorf("origin %q: ACAO = %q", origin, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestStreamServer_HLS_InvalidSessionID verifica que el hlsHandler rechaza
|
||||
// session IDs con caracteres ilegales devolviendo 404 (uniforme con sesión
|
||||
// inexistente) para no filtrar el formato aceptado a un attacker.
|
||||
|
|
|
|||
|
|
@ -10,3 +10,39 @@ import "regexp"
|
|||
// anything containing slashes, dots, or path separators is rejected so a
|
||||
// compromised or buggy server cannot escape hlsTmpDirRoot via os.MkdirAll.
|
||||
var validSessionID = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,128}$`)
|
||||
|
||||
// defaultCORSAllowedOrigins is the baseline of browser origins that may
|
||||
// XHR-probe `/health` and friends on the local daemon. Production hosts are
|
||||
// hardcoded; localhost on the dev port used by torrentclaw-web is included
|
||||
// so dev builds work without extra configuration. Operators may add more
|
||||
// origins via the [downloads] cors_extra_origins TOML key.
|
||||
//
|
||||
// The dev port matches `next dev -p 3030` in torrentclaw-web/package.json.
|
||||
// 127.0.0.1 is listed in addition to localhost because some browsers treat
|
||||
// them as distinct origins for CORS.
|
||||
//
|
||||
// Note: media tags (<video src>, <audio src>) do not send the Origin
|
||||
// header so they are not gated by CORS at all; this allowlist only
|
||||
// affects fetch()/XHR.
|
||||
var defaultCORSAllowedOrigins = []string{
|
||||
"https://torrentclaw.com",
|
||||
"https://app.torrentclaw.com",
|
||||
"http://localhost:3030",
|
||||
"http://127.0.0.1:3030",
|
||||
}
|
||||
|
||||
// buildCORSAllowlist merges the default origins with any extras supplied by
|
||||
// the operator. Returned map is intended to be installed once at Listen()
|
||||
// and treated as read-only afterwards.
|
||||
func buildCORSAllowlist(extra []string) map[string]struct{} {
|
||||
out := make(map[string]struct{}, len(defaultCORSAllowedOrigins)+len(extra))
|
||||
for _, o := range defaultCORSAllowedOrigins {
|
||||
out[o] = struct{}{}
|
||||
}
|
||||
for _, o := range extra {
|
||||
if o != "" {
|
||||
out[o] = struct{}{}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
|
|
@ -85,15 +86,22 @@ func extractZip(archivePath, destDir string) (string, error) {
|
|||
|
||||
target := binaryName + ".exe"
|
||||
|
||||
for _, f := range r.File {
|
||||
name := filepath.Base(f.Name)
|
||||
// Resolve destDir to its absolute form once so the ZIP-slip check below
|
||||
// can compare canonical paths instead of fragile substring matches.
|
||||
absDest, err := filepath.Abs(destDir)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("resolve dest: %w", err)
|
||||
}
|
||||
|
||||
// Guard against path traversal
|
||||
if strings.Contains(f.Name, "..") {
|
||||
for _, f := range r.File {
|
||||
if f.FileInfo().IsDir() {
|
||||
continue
|
||||
}
|
||||
|
||||
if name != target {
|
||||
if filepath.Base(f.Name) != target {
|
||||
continue
|
||||
}
|
||||
absDst, ok := safeZipPath(f.Name, target, absDest)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
@ -102,8 +110,7 @@ func extractZip(archivePath, destDir string) (string, error) {
|
|||
return "", err
|
||||
}
|
||||
|
||||
dst := filepath.Join(destDir, target)
|
||||
out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755)
|
||||
out, err := os.OpenFile(absDst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o755)
|
||||
if err != nil {
|
||||
rc.Close()
|
||||
return "", err
|
||||
|
|
@ -116,8 +123,41 @@ func extractZip(archivePath, destDir string) (string, error) {
|
|||
}
|
||||
out.Close()
|
||||
rc.Close()
|
||||
return dst, nil
|
||||
return absDst, nil
|
||||
}
|
||||
|
||||
return "", fmt.Errorf("binary %q not found in archive", target)
|
||||
}
|
||||
|
||||
// safeZipPath validates that a ZIP entry name is safe to extract under
|
||||
// absDest, then returns the absolute destination path (always
|
||||
// absDest/target, never the raw entry name — we still only extract files
|
||||
// matched by Base name).
|
||||
//
|
||||
// Rejected: absolute paths, paths that resolve to "..", paths containing
|
||||
// a "../" or "..\\" component, and any entry whose final destination
|
||||
// would land outside absDest. The check uses path.Clean on the entry's
|
||||
// native separator (ZIP uses forward slashes by spec, but some authors
|
||||
// emit backslashes — we treat both as separators here so a hostile entry
|
||||
// on Linux can't bypass the substring scan).
|
||||
func safeZipPath(entryName, target, absDest string) (string, bool) {
|
||||
// Normalise both separators to "/" so the check works on Linux too,
|
||||
// where filepath.Separator is "/" and a hostile "..\\foo" string is
|
||||
// otherwise treated as a single filename component by filepath.Clean.
|
||||
normalised := strings.ReplaceAll(entryName, `\`, "/")
|
||||
cleaned := path.Clean(normalised)
|
||||
if cleaned == ".." ||
|
||||
strings.HasPrefix(cleaned, "../") ||
|
||||
strings.Contains(cleaned, "/../") ||
|
||||
path.IsAbs(cleaned) {
|
||||
return "", false
|
||||
}
|
||||
absDst, err := filepath.Abs(filepath.Join(absDest, target))
|
||||
if err != nil {
|
||||
return "", false
|
||||
}
|
||||
if !strings.HasPrefix(absDst+string(filepath.Separator), absDest+string(filepath.Separator)) {
|
||||
return "", false
|
||||
}
|
||||
return absDst, true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1085,3 +1085,40 @@ func TestDownloadSetsUserAgent(t *testing.T) {
|
|||
t.Errorf("User-Agent = %q, want 'unarr-updater'", gotUA)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSafeZipPath(t *testing.T) {
|
||||
dest := t.TempDir()
|
||||
absDest, err := filepath.Abs(dest)
|
||||
if err != nil {
|
||||
t.Fatalf("abs dest: %v", err)
|
||||
}
|
||||
|
||||
// Names that must extract successfully.
|
||||
good := []string{
|
||||
"unarr.exe",
|
||||
"bin/unarr.exe",
|
||||
"./unarr.exe",
|
||||
"folder/sub/unarr.exe",
|
||||
}
|
||||
for _, name := range good {
|
||||
if _, ok := safeZipPath(name, "unarr.exe", absDest); !ok {
|
||||
t.Errorf("safeZipPath(%q) = ok:false, want ok:true", name)
|
||||
}
|
||||
}
|
||||
|
||||
// Names that must be rejected for path-traversal reasons.
|
||||
bad := []string{
|
||||
"../unarr.exe",
|
||||
"..",
|
||||
"foo/../../unarr.exe",
|
||||
"/etc/passwd",
|
||||
"/abs/unarr.exe",
|
||||
`..\..\windows\system32\unarr.exe`, // backslash entries that escape
|
||||
"../../bin/unarr.exe",
|
||||
}
|
||||
for _, name := range bad {
|
||||
if _, ok := safeZipPath(name, "unarr.exe", absDest); ok {
|
||||
t.Errorf("safeZipPath(%q) = ok:true, want ok:false", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue