File-event hooks substitute attacker-controlled filenames into command strings before shell parsing, enabling low-privilege command injection that bypasses Perm.Execute

Status

Repository / Component

Plain-English Description

When file-event hooks are turned on, the server pastes the name of the file a user just uploaded, renamed, or deleted directly into a command that runs on the server. A user who simply names their file with shell tricks can make the server run their own commands, even without permission to run commands.

Description of the Underlying Issue

When EnableExec is on, resource operations call runner.exec via RunHook (http/resource.go:114 delete, :162 upload, :199 save, :257 patch rename/copy, and http/tus_handlers.go), passing the request path (an attacker-chosen filename derived from user.FullPath, users/users.go:103-105). In runner/runner.go:84-90, os.Expand rewrites every command token, textually substituting FILE/DESTINATION into the argv BEFORE any shell parsing; os.Expand is oblivious to shell quoting, so an operator's defensive "$FILE" or '$FILE' is broken by a filename that supplies the matching quote plus metacharacters. runner/runner.go:92 then executes the expanded argv with exec.Command. Reaching a shell is not conditioned on settings.Shell: in shell mode (runner/parser.go:16-22) the entire raw hook string carrying the $FILE token is appended and passed to the shell after substitution; in default mode a shell-wrapper hook such as sh -c 'cp "$FILE" /backup/' (parsed by shlex at runner/commands.go:34-64) routes the substituted token through the shell it invokes. The env-var channel at runner/runner.go:93-97 already exposes these same values safely, making the os.Expand-into-argv step redundant. Crucially the hook path is gated only by EnableExec plus ordinary file-op permissions (Create/Modify/Delete); Perm.Execute is never checked, whereas the interactive command endpoint (http/commands.go:64) does require it — a proven privilege escalation from file-write to code execution.

Potential Attack

Deployment has EnableExec=true with settings.Shell=[/bin/bash,-c] (or default Shell plus a wrapper hook like sh -c 'echo $FILE') and a before_upload hook 'echo $FILE'. A non-admin user holding only Perm.Create uploads a file named x$(id).txt via POST /api/resources/x$(id).txt. resource.go:162 calls RunHook('upload', r.URL.Path, ...); ParseCommand yields [/bin/bash,-c,'echo $FILE'] (or [sh,-c,'echo $FILE'] for a wrapper); os.Expand rewrites the -c body to 'echo /scope/x$(id).txt'; exec.Command runs the shell, which executes the embedded id command. The same works via rename/copy/delete/save/tus hooks with DESTINATION/FILE.

Outcomes of Potential Attack

Arbitrary command execution on the server host under the FileBrowser process account, reached by a user who has only file-write permissions and explicitly lacks Perm.Execute. This is remote code execution and full privilege escalation: proxy/self-signup users (auth/proxy.go:50, http/auth.go:177 set Perm.Execute=false) are a concrete low-privilege population that can still trigger hooks. Consequences include reading/altering all server-side data, lateral movement, and persistence.

Affected Scope

runner.exec (runner/runner.go:55-118) reached via RunHook from resource delete/upload/save/patch(rename,copy) and tus upload; any authenticated user holding Perm.Create/Modify/Delete (Perm.Execute NOT required) when EnableExec is on and a $FILE/$DESTINATION hook reaches a shell.

Suggested Fix (plain english)

Stop pasting user-supplied filenames into hook command text.

Suggested Fix (detailed)

Remove the os.Expand argv-substitution loop at runner/runner.go:84-90 and deliver FILE/SCOPE/TRIGGER/USERNAME/DESTINATION to hooks exclusively via cmd.Env (already populated at runner.go:93-97), so untrusted values can never be parsed as command syntax. If command-string templating must remain, shell-quote every substituted value for the target shell and never substitute into argv[0]. Defense in depth: gate hook execution behind Perm.Execute (or a dedicated hook permission) rather than EnableExec alone; update www/docs/command-execution.md to stop teaching raw $FILE-in-command and show env-var usage instead. Verify by uploading a file named x$(id).txt with a before_upload 'echo $FILE' hook and confirming no command substitution occurs. This record supersedes the narrower shell-mode-only DOK-100018. Remove the os.Expand argv-substitution loop (runner/runner.go:84-90) and pass FILE/SCOPE/TRIGGER/USERNAME/DESTINATION to hooks only via the environment (already done at 93-97) so untrusted values can never be parsed as command syntax. If command-string templating must remain, shell-quote every substituted value for the target shell and never substitute into argv[0]. Defense in depth: gate hook execution behind Perm.Execute (or a dedicated hook permission) rather than EnableExec alone; update www/docs/command-execution.md to stop teaching raw $FILE-in-command and show env-var usage instead.

Validation

Canonical, broader command-injection finding. Every cited line verified verbatim; the os.Expand-before-shell-parse sink, the shell-reachability condition (settings.Shell OR shell-wrapper hook), and the Perm.Execute bypass are all complete in source. Fully_valid: the vulnerable path is proven end-to-end; the only unresolved item is whether a given install enabled the feature, which is a config state, not a reachability gap. Supersedes the narrower shell-mode-only DOK-100018.

first_opinion

Full Evidence

runner/runner.go:84-90 — os.Expand(arg, envMapping) rewrites every command[i>=1] token, substituting FILE/DESTINATION (attacker-chosen filename via user.FullPath) into the argv BEFORE exec

runner/runner.go:92 — exec.Command(command[0], command[1:]...) executes the expanded argv; when command[0] is a shell the expanded token is the shell's -c script body

runner/runner.go:93-97 — the same FILE/SCOPE/TRIGGER/USERNAME/DESTINATION values are ALSO exported via cmd.Env, a safe channel that makes the os.Expand loop removable

runner/parser.go:16-22 — shell mode (settings.Shell non-empty) appends the whole raw hook string (carrying the literal $FILE token) as command[len], so os.Expand then injects the attacker path into that shell script; default mode splits argv but a shell-wrapper hook still yields command[0]=sh

runner/commands.go:34-64 — SplitCommandAndArgs uses shlex.Split, so an admin hook 'sh -c \'cp "$FILE" /backup/\'' parses to argv [sh,-c,cp "$FILE" /backup/]; os.Expand then edits the sh -c body

http/resource.go:114 (delete, gated Perm.Delete@87), :162 (upload, Perm.Create@128), :199 (save, Perm.Modify@182), :257 (patch rename/copy) and http/tus_handlers.go — all call d.RunHook with r.URL.Path (attacker filename)

http/commands.go:64 — the explicit command endpoint requires !d.server.EnableExec || !d.user.Perm.Execute; the hook path enforces neither, proving hook exec bypasses the Perm.Execute control

http/data.go:63 — Runner.Enabled = server.EnableExec (no per-user gate)

users/users.go:103-105 — FullPath = afero.FullBaseFsPath, so FILE/DESTINATION embed the attacker-chosen filename verbatim

settings/storage.go:94-95 Shell defaults to []; cmd/config.go:38 exposes a documented '--shell' flag; www/docs/command-execution.md:5,9,28 documents EnableExec-off-by-default (known vulns, issue #5199) yet teaches 'execute any shell command' with 'before_copy "echo $FILE"'

auth/proxy.go:50 and http/auth.go:177 set Perm.Execute=false for proxy/self-signup users, who can still trigger hooks — concrete privilege-escalation population

Proven fact: os.Expand at runner.go:84-90 performs textual substitution of attacker-controlled FILE/DESTINATION into argv tokens before any shell parsing; os.Expand ignores shell quoting, so an admin's defensive "$FILE"/'$FILE' is broken by a filename containing the matching quote plus metacharacters.

Proven fact: In shell mode the entire raw hook (with the $FILE token) is passed to the shell after substitution, so any $FILE/$DESTINATION-referencing hook is injectable (e.g. filename x$(id).txt executes id).

Proven fact: In default mode (Shell=[]) a shell-wrapper hook (sh -c/bash -c '... $FILE ...') routes the substituted token through the shell it invokes, so injection is not conditioned on settings.Shell — correctly overturning the dismissed concern whose premise 'exec.Command uses no shell' fails for shell-wrapper hooks.

Proven fact: The hook path is gated only by EnableExec plus ordinary file-op permissions (Create/Modify/Delete); Perm.Execute is never checked on it, while the interactive command endpoint (commands.go:64) does require it — a proven privilege escalation from file-write to code execution.

Proven fact: The env-var channel at runner.go:93-97 already exposes these values safely (shell parameter expansion of an env var is not re-parsed for command substitution), so the os.Expand-into-argv step is redundant and is the sole source of the injection.

Unvalidated fact: Whether any given deployment has turned EnableExec on and configured a before_/after_ hook that both references $FILE/$DESTINATION and reaches a shell (settings.Shell set, or a sh -c wrapper). This is a runtime configuration state, not a code fact; it does not affect the presence of the sink, only whether a specific install is currently exploitable.

runner/runner.go:84-90 — os.Expand(arg, envMapping) rewrites every command[i>=1] token, substituting FILE/DESTINATION (attacker-chosen filename via user.FullPath) into the argv before exec

runner/parser.go:16-22 — shell mode appends the whole raw hook string (with the literal $FILE token) as command[len]; default mode splits argv but a shell-wrapper hook still yields command[0]=sh

runner/commands.go:34-64 — SplitCommandAndArgs uses shlex.Split, so sh -c 'cp "$FILE" /backup/' parses to [sh,-c,cp "$FILE" /backup/] and os.Expand then edits the sh -c body

http/resource.go:114 (delete, gated Perm.Delete@87), :162 (upload, Perm.Create@128), :199 (save, Perm.Modify@182), :257 (patch rename/copy) and http/tus_handlers.go — all call d.RunHook with r.URL.Path

http/commands.go:64 — the explicit command endpoint requires !EnableExec || !Perm.Execute; the hook path enforces neither, proving hook exec bypasses the Perm.Execute control

auth/proxy.go:50 and http/auth.go:177 set Perm.Execute=false for proxy/self-signup users, who can still trigger hooks

www/docs/command-execution.md:5,9,28 documents EnableExec off-by-default (known vulns, issue #5199) yet teaches 'execute any shell command' with 'before_copy "echo $FILE"'

Proven fact: os.Expand at runner.go:84-90 performs textual substitution of attacker-controlled FILE/DESTINATION into argv tokens before any shell parsing; it ignores shell quoting, so a defensive "$FILE"/'$FILE' is broken by a filename containing the matching quote plus metacharacters.

Proven fact: In default mode a shell-wrapper hook (sh -c/bash -c '... $FILE ...') routes the substituted token through the shell it invokes, so injection is not conditioned on settings.Shell.

Proven fact: The hook path is gated only by EnableExec plus Create/Modify/Delete; Perm.Execute is never checked, while the interactive command endpoint (commands.go:64) does require it — a proven escalation from file-write to code execution.

Proven fact: The env-var channel at runner.go:93-97 already exposes these values safely, so the os.Expand-into-argv step is redundant and is the sole source of the injection.

Unvalidated fact: Whether a given deployment has turned EnableExec on and configured a before_/after_ hook that both references $FILE/$DESTINATION and reaches a shell (settings.Shell set, or a sh -c wrapper). This is a runtime configuration state, not a code fact; it affects only whether a specific install is currently exploitable, not the presence of the sink.

runner/runner.go:84-90 os.Expand substitutes attacker FILE/DESTINATION into hook argv before exec

runner/runner.go:92 exec.Command(command[0], command[1:]...) executes the expanded argv; when argv[0] is a shell, the expanded token is a shell script

runner/runner.go:93-97 the same values are ALSO exported as environment variables — a safe channel already present, making the os.Expand loop removable

runner/parser.go:16-22 default mode (Shell=[]) still yields a shell-wrapper argv when the admin hook is 'sh -c ...'; whole-raw-to-shell in Shell mode

http/resource.go:114,162,199,257 and http/tus_handlers.go:234 RunHook reachable with Perm.Create/Modify/Delete, not Perm.Execute

http/data.go:63 Runner Enabled from server.EnableExec only

www/docs/command-execution.md:9,19,28 documents $FILE inside the command string and 'execute any shell command' — the exact unsafe idiom

www/docs/command-execution.md:3-5 EnableExec off by default since v2.33.8 with explicit security caution (mitigating context, not a fix)

No filename metacharacter validation in http/ or files/ write/copy/rename paths (grep negative)

Contradicts dismissed-concerns.jsonl entry 'Hook/command injection in default (non-shell) mode' whose premise 'exec.Command uses no shell' fails for shell-wrapper hooks; expands prior candidate 'Low-privilege RCE via crafted filename injected into file-event hooks in shell mode'