DOKIMASecurity review report

Overview

File-event hook subprocesses spawn without concurrency, timeout, or Perm.Execute gating (resource abuse)

medium Possibly Valid medium confidence

DOK-100120-FILEBROWSER-COMMAND-EXECUTION-FILE-EVENT-HOOKS-S · filebrowser · command-execution

Status

Possibly Valid

Missing resource controls on hook subprocess spawning and the Perm.Execute-gate bypass are proven in source, but the vulnerability only materializes when the operator has configured file-event hooks and depends on those hooks' cost/blocking mode - runtime configuration facts, hence possibly_valid.

Repository / Component

Repository
filebrowser
Component
command-execution

Plain-English Description

When the administrator has set up commands that run automatically on file changes, any user allowed to create or edit files can trigger those commands over and over with no limit. Rapid file operations can pile up unlimited background processes and slow down or crash the server.

Description of the Underlying Issue

The hook runner (runner.Runner.exec, runner/runner.go:55-118) launches each configured before_/after_ file-event command with exec.Command (runner/runner.go:92) using no context deadline (no exec.CommandContext), no concurrency cap, and no rate limit. When a command is suffixed with '&' the blocking flag becomes false and the runner calls cmd.Start() while detaching cmd.Wait() to a goroutine (runner/runner.go:103-113), so the triggering request returns immediately and the spawned process is no longer bound to request lifetime or any backpressure. Hooks are gated only by the global EnableExec flag (runner/runner.go:25,41; http/data.go:63 Runner{Enabled: server.EnableExec}; cmd/root.go:353 EnableExec defaults true), and they are invoked from ordinary mutating resource handlers whose permission checks are Create/Modify (http/resource.go:128/182, RunHook at :114/:162/:199/:257). This bypasses the Perm.Execute gate that the explicit /api/command endpoint enforces (http/commands.go:64).

Potential Attack

An authenticated user holding only Create/Modify/Rename/Delete file permissions (not Perm.Execute) issues a high-rate stream of mutating file operations (uploads, renames, deletes, tus completions). Each operation invokes RunHook, spawning one subprocess per configured hook. With '&'-suffixed hooks the requests return immediately while Wait is detached, so the attacker is not throttled by request latency and can accumulate processes far faster than they complete.

Outcomes of Potential Attack

Unbounded or abandoned process accumulation exhausting CPU, memory, PIDs, and file descriptors on the host, degrading or denying service to the filebrowser process and potentially the whole machine. The user also effectively drives command execution without holding Perm.Execute, defeating the intended capability separation for command running.

Affected Scope

runner.Runner.exec (runner/runner.go:55-118); file-event hooks invoked from mutating resource handlers (POST/PUT/PATCH/DELETE, http/resource.go:114/162/199/257) and tus upload completion; triggerable by any authenticated user with Create/Modify/Rename/Delete file permissions (Perm.Execute NOT required).

Suggested Fix (plain english)

Run file-event hooks through a bounded worker pool with per-command timeouts, cap how many can run or queue at once, and don't fully detach background commands.

Suggested Fix (detailed)

Replace exec.Command with exec.CommandContext bound to a per-exec timeout in runner.Runner.exec. Introduce a bounded semaphore/worker pool limiting concurrent and queued hook processes, applied to both blocking and nonblocking ('&') paths; for nonblocking hooks, supervise cmd.Wait() within the pool rather than detaching it to an unmanaged goroutine so process counts are capped and reaped. Add per-user rate limiting on mutating handlers that call RunHook. Consider gating command-bearing hooks behind Perm.Execute (or a dedicated capability) so that triggering process spawning requires the same privilege as /api/command. Verify by configuring an '&'-suffixed hook and confirming that a burst of file operations does not exceed the configured concurrent-process cap and that abandoned processes are reaped. Wrap hook execution in a bounded worker pool/semaphore with a per-exec context timeout (exec.CommandContext), cap concurrent and queued hook processes, supervise nonblocking hooks instead of fully detaching them, and consider requiring Perm.Execute (or a dedicated capability) to trigger command-bearing hooks.

Validation

Missing resource controls on hook subprocess spawning and the Perm.Execute-gate bypass are proven in source, but the vulnerability only materializes when the operator has configured file-event hooks and depends on those hooks' cost/blocking mode - runtime configuration facts, hence possibly_valid.

first_opinion

Full Evidence

runner/runner.go:92 exec.Command(command[0], command[1:]...) - no CommandContext/timeout
runner/runner.go:58-61 '&' suffix -> blocking=false; :103-113 nonblocking branch: defer go cmd.Wait(); return cmd.Start() (no backpressure, no cap)
runner/runner.go:25,41 hooks gated only by r.Enabled; http/data.go:63 Runner{Enabled: server.EnableExec}; cmd/root.go:353 EnableExec = !disableExec (default true)
http/resource.go:128/182 perms checked are Create/Modify (not Execute); RunHook at :114/:162/:199/:257
http/commands.go:64 the explicit /api/command endpoint requires EnableExec && Perm.Execute - hooks bypass that Execute gate
cmd/root.go:246-248 http.Server sets only ReadHeaderTimeout
Proven fact: Hook subprocesses are spawned with exec.Command with no context/timeout, no concurrency cap, and no rate limit; '&'-suffixed hooks call cmd.Start and detach cmd.Wait to a goroutine, removing request backpressure.
Proven fact: Hooks are gated only by EnableExec, not Perm.Execute, and are wrapped around ordinary file mutations, so a user without Perm.Execute can drive process spawning - unlike the /api/command endpoint which does require Perm.Execute.
Unvalidated fact: Whether the operator has configured any before_/after_ hook commands (settings.Commands) - RunHook is a no-op when none are configured; and whether configured hooks are resource-intensive or use the '&' suffix. These are runtime configuration values not present in source.
runner/runner.go:92 exec.Command(command[0], command[1:]...) — no CommandContext/timeout
runner/runner.go:103-113 nonblocking branch: defer go cmd.Wait(); return cmd.Start() (no backpressure, no cap)
http/commands.go:64 /api/command requires EnableExec && Perm.Execute — hooks bypass that Execute gate
Proven fact: Hooks are gated only by EnableExec, not Perm.Execute, and are wrapped around ordinary file mutations, so a user without Perm.Execute can drive process spawning — unlike /api/command which requires Perm.Execute.
Unvalidated fact: Whether the operator has configured any before_/after_ hook commands (settings.Commands) — RunHook is a no-op when none are configured — and whether configured hooks are resource-intensive or use the '&' suffix. These are runtime configuration values not present in source.
runner/runner.go:92 cmd := exec.Command(command[0], command[1:]...) — no CommandContext/timeout
runner/runner.go:58-61 '&' suffix -> blocking=false
runner/runner.go:103-114 nonblocking branch: defer go cmd.Wait(); return cmd.Start() (no backpressure, no cap)
runner/runner.go:25,41 r.Enabled gate = EnableExec only
http/data.go:63 Runner{Enabled: server.EnableExec}
http/resource.go:128/182/243-259 perms checked are Create/Modify/Rename — not Execute; RunHook at 114/162/199/257
http/tus_handlers.go:234 RunHook 'upload' reachable on tus write completion
cmd/root.go:246-248 http.Server sets only ReadHeaderTimeout:60s
grep: no semaphore/rate-limit/CommandContext/Process.Kill in runner/ or http/