Username-enumeration timing side channel in hook 'pass' auth action (missing dummyHash equalization)

Status

Repository / Component

Plain-English Description

When the app is configured to check passwords through an external hook, a login attempt for a real username takes measurably longer than one for a username that does not exist. An attacker timing these responses could learn which usernames are valid, even without guessing any password.

Description of the Underlying Issue

In the hook authentication path, the 'pass' action verifies credentials locally. At auth/hook.go:70-75 the code runs `u, err := a.Users.Get(...); if err != nil || !users.CheckPwd(a.Cred.Password, u.Password) { return nil, os.ErrPermission }`. Because of Go's short-circuit `||`, when the username does not exist `err != nil` is true and the expensive `users.CheckPwd` (bcrypt.CompareHashAndPassword at DefaultCost, users/password.go:32-34, ~tens of ms) is never executed. For an existing username, bcrypt runs. This produces a timing asymmetry between valid and invalid usernames. The primary JSON auth path deliberately closes exactly this channel: auth/json.go:17-19 defines a `dummyHash` (commented as preventing user-enumeration timing attacks) and json.go:58-71 assigns it on the Get-error branch and then calls `users.CheckPwd` unconditionally so bcrypt runs in both cases. The hook 'pass' branch omits this equalization — a developer-acknowledged inconsistency in a control the codebase already implements elsewhere.

Potential Attack

With the deployment configured for AuthMethod=hook and a hook that returns action=pass (delegating to FileBrowser's local bcrypt check), an unauthenticated attacker submits many /api/login requests alternating a candidate username that may exist against a username known to be absent, using any password. By statistically comparing mean response latency across many samples, the attacker infers which usernames execute the bcrypt path and are therefore valid accounts, building a list of real usernames to target with password guessing or credential-stuffing.

Outcomes of Potential Attack

The attacker gains confirmation of valid vs. invalid usernames (account enumeration) without needing any correct password, narrowing later brute-force/credential-stuffing to real accounts and aiding social-engineering. No authentication bypass or data disclosure results directly; the gain is reconnaissance of the user namespace.

Affected Scope

auth/hook.go:70-75 (hook 'pass' action local credential check). The Get-error branch short-circuits before bcrypt via Go's || operator, omitting the dummyHash timing-equalization that auth/json.go:58-71 applies — leaking username validity via /api/login response latency when AuthMethod=hook and the hook returns action=pass.

Suggested Fix (plain english)

Make the hook 'pass' login path spend the same time whether or not the username exists, exactly as the built-in password path already does.

Suggested Fix (detailed)

Mirror auth/json.go: in the Get-error branch of the hook 'pass' action (auth/hook.go:70-75), when a.Users.Get returns an error, call users.CheckPwd(a.Cred.Password, dummyHash) (the same constant defined in auth/json.go:17-19) before returning os.ErrPermission, so bcrypt executes for absent and existing users alike. Verify by confirming that the bcrypt comparison is always performed regardless of Get outcome, equalizing response time. Consider factoring the dummyHash equalization into a shared helper used by both json and hook paths to prevent future divergence. Mirror json.go: on the Get-error branch of the hook 'pass' action, call users.CheckPwd(a.Cred.Password, dummyHash) before returning os.ErrPermission so bcrypt runs for absent and existing users alike, equalizing response time.

Validation

The timing asymmetry and the missing dummyHash equalization are undeniable from reading (hook.go:72 short-circuit vs json.go:58-71), and the codebase's own dummyHash pattern establishes this class as a real, intended-to-be-mitigated threat. possibly_valid with low confidence because practical exploitability (measurability of the bcrypt delta against the dominating, variable common-mode hook subprocess and network noise) is a runtime fact that source cannot settle.

first_opinion

Full Evidence

auth/hook.go:70-75 case "pass": u,err:=a.Users.Get(...); if err!=nil || !users.CheckPwd(a.Cred.Password,u.Password){return nil,os.ErrPermission} — for an absent user err!=nil is true, so || short-circuits and users.CheckPwd (bcrypt) is never called; for an existing user CheckPwd runs

auth/json.go:17-19 dummyHash const documented 'used to prevent user enumeration timing attacks'; json.go:58-71 sets hash=dummyHash on Get-error then calls users.CheckPwd unconditionally — the equalization the hook path omits

users/password.go:32-34 CheckPwd -> bcrypt.CompareHashAndPassword; password.go:27 HashPwd uses bcrypt.DefaultCost (~tens of ms per compare) — the measurable per-call cost that is present only for existing users

auth/hook.go:56,106-109 a.RunCommand() (exec.Command(...).Output()) runs for both absent and existing users before the switch, so the subprocess cost is common-mode

Proven fact: The hook 'pass' branch has a genuine timing asymmetry: absent usernames return without executing bcrypt, existing usernames execute bcrypt.CompareHashAndPassword at DefaultCost (read directly from hook.go:72 + password.go:33).

Proven fact: The primary JSON auth path deliberately equalizes this exact channel with dummyHash (json.go:17-19,58-71), and the hook 'pass' path omits the equivalent hardening — a concrete, developer-acknowledged inconsistency.

Proven fact: The hook subprocess (RunCommand) executes for both absent and existing users, so it is common-mode; the residual code-level delta is the bcrypt call.

Unvalidated fact: Whether the ~tens-of-ms bcrypt delta is actually distinguishable in practice is a runtime/measurement fact not settleable from source: it depends on the variance of the common-mode hook subprocess (process spawn plus the hook's own, possibly username-dependent, work such as an external auth lookup) and on network jitter, which can dominate or confound the signal.

Unvalidated fact: Requires the deployment to configure AuthMethod=hook with a hook that emits action=pass (delegating to filebrowser's local bcrypt check) rather than auth/block.

auth/hook.go:70-75 case "pass": `u, err := a.Users.Get(a.Server.Root, a.Cred.Username); if err != nil || !users.CheckPwd(a.Cred.Password, u.Password) { return nil, os.ErrPermission }` — verified in source; `||` short-circuits so CheckPwd (bcrypt) runs only for existing users.

auth/json.go:17-19 `dummyHash` constant documented as preventing user-enumeration timing attacks; json.go:58-71 sets hash=dummyHash on Get-error then calls users.CheckPwd unconditionally — the equalization the hook path omits.

users/password.go:32-34 CheckPwd -> bcrypt.CompareHashAndPassword; password.go:27 HashPwd uses bcrypt.DefaultCost (~tens of ms per compare) — the measurable per-call cost present only for existing users.

auth/hook.go:56,106-109 a.RunCommand() (exec.Command(...).Output()) runs for both absent and existing users before the switch, so the subprocess cost is common-mode.

Proven fact: The hook 'pass' branch has a genuine timing asymmetry: absent usernames return without executing bcrypt; existing usernames execute bcrypt.CompareHashAndPassword at DefaultCost (hook.go:72 short-circuit + password.go:33).

Proven fact: The primary JSON auth path deliberately equalizes this exact channel with dummyHash (json.go:17-19,58-71); the hook 'pass' path omits the equivalent hardening — a concrete, developer-acknowledged inconsistency.

Unvalidated fact: Whether the ~tens-of-ms bcrypt delta is distinguishable in practice is a runtime/measurement fact not settleable from source: it depends on the variance of the common-mode hook subprocess (process spawn plus the hook's own, possibly username-dependent, work) and on network jitter, which can dominate or confound the signal.

Unvalidated fact: Exploitability requires the deployment to configure AuthMethod=hook with a hook that emits action=pass (delegating to FileBrowser's local bcrypt check) rather than auth/block or auth/auth.

auth/hook.go:70-75 case "pass": u,err:=a.Users.Get(a.Server.Root,a.Cred.Username); if err!=nil || !users.CheckPwd(a.Cred.Password,u.Password){return nil,os.ErrPermission} — absent user returns before bcrypt via || short-circuit

auth/json.go:17-19 dummyHash const documented 'used to prevent user enumeration timing attacks'; json.go:58-71 hash=dummyHash on Get-error then CheckPwd runs unconditionally — the equalization the hook path omits

users/password.go:32-34 CheckPwd -> bcrypt.CompareHashAndPassword; password.go:27 HashPwd uses bcrypt.DefaultCost (the measurable per-call cost)

auth/hook.go:56 RunCommand + :106-109 exec.Command(...).Output() run for both existing and absent users (common-mode, cancels in the timing difference)

Canonical-state check: 0 hits for dummyHash/'timing side channel'/'user enumeration' in candidate-findings.jsonl and likely-findings.jsonl; the only prior hook-'pass' dismissal ('Nil-pointer / auth bypass in hook pass action when user is absent') covers ONLY bypass+nil-deref, not timing; rejected-observations dummyHash/timing hits belong to other components (search, settings-config)