Unescaped attacker-controlled path logged with %s in raw download handler enables log injection (CWE-117)

Status

Repository / Component

Plain-English Description

A file download URL lets a caller slip newline characters into the requested path. When the download fails, the server writes that path straight into its log without cleaning it, so an attacker can add fake lines to the log. On a password-less public share this works without even logging in.

Description of the Underlying Issue

parseQueryFiles (http/raw.go:29-41) takes the attacker-controlled ?files= query value, url.QueryUnescape-decodes it (so %0a becomes a literal newline), then runs it through slashClean/gopath.Clean/filepath.Join — none of which strip CR/LF control bytes. The cleaned path reaches log.Printf("Failed to get files from %s: %v", fname, err) at http/raw.go:181, which uses %s rather than %q/strconv.Quote, so the newline is written verbatim into the Go standard-logger stream. The error branch is trivially reachable: any nonexistent crafted path passes the purely rule-based d.Check (http/data.go:29-48 rejects deny/hidden globs but not control characters) and then fails Fs.Stat, driving the log call. http/raw.go:154 is a secondary %s sink over directory-entry names. The broken invariant is that untrusted input must be neutralized before being emitted to a log sink.

Potential Attack

An attacker requests a nonexistent path whose ?files= value contains a percent-encoded newline followed by a forged log record, e.g. GET /api/public/dl/{hash}?files=ghost%0a2026-01-01T00:00:00Z%20INFO%20forged-admin-login. Via publicDlHandler->rawDirHandler (http/public.go:120-127) this reaches the raw handler with no authentication when the share's PasswordHash is empty (authenticateShareRequest returns (0,nil), public.go:129-132). getFiles passes d.Check, Fs.Stat fails, and raw.go:181 logs the crafted path, emitting the attacker's second line into the log.

Outcomes of Potential Attack

The attacker forges or splits log records: injecting fake log lines (spoofing events such as an admin login) into line-oriented log parsers, or embedding ANSI/terminal escape sequences that alter what an operator sees when viewing logs in a terminal. This undermines the integrity and trustworthiness of the audit trail; it does not by itself grant code execution or data access.

Affected Scope

http/raw.go:181 (log.Printf %s on fname) and http/raw.go:154 (log.Printf %s on fPath); reachable via GET /api/raw/<dir>?files=... (authenticated Perm.Download) and GET /api/public/dl/{hash}?files=... (unauthenticated for password-less directory shares).

Suggested Fix (plain english)

Escape or strip control characters from any path before writing it to a log.

Suggested Fix (detailed)

Change http/raw.go:181 and http/raw.go:154 to log the path with %q / strconv.Quote(fname) (Go's %q escapes \n, \r and ANSI bytes), or strip CR/LF before logging. Apply the same neutralization to the analogous request-path loggers at http/data.go:71 and http/commands.go:34, which also emit r.URL.Path with %s. Verify by confirming no log call in the http package prints attacker-derived path or filename data with an unescaped %s verb. Neutralize control characters before logging in both sinks: use %q / strconv.Quote(fname) (Go's %q escapes \n, \r, and ANSI bytes) at http/raw.go:181 and http/raw.go:154, or strip CR/LF. Apply the same to the analogous request-path loggers at http/data.go:71 and http/commands.go:34, which also use %s on r.URL.Path.

Validation

Complete, reachable log-injection path proven end-to-end in source: attacker-controlled ?files= newline survives path cleaning and is emitted unescaped via %s at raw.go:181, unauthenticated through password-less public directory shares. Low severity (log forgery) but fully evidenced; one supporting evidence bullet about data.go %q is inaccurate and noted, without affecting the core sink.

first_opinion

Full Evidence

http/raw.go:29,35,40-41 parseQueryFiles: r.URL.Query().Get("files") is split, url.QueryUnescape decodes %0a to a literal newline, then slashClean->gopath.Clean and filepath.Join preserve the control byte (path.Clean does not strip \n/\r)

http/raw.go:181 log.Printf("Failed to get files from %s: %v", fname, err) — fname is the attacker-derived path, emitted with %s (unescaped)

http/raw.go:112-120,152-155 getFiles: d.Check(path) returns nil,nil on a scope failure (no log), but a nonexistent crafted path passes d.Check then Fs.Stat errors, returning (nil,err) and driving the raw.go:181 error branch; raw.go:154 is a secondary %s sink over directory-entry names

http/public.go:120-127 publicDlHandler->rawDirHandler and public.go:129-132 authenticateShareRequest returns (0,nil) when link.PasswordHash=="" — the sink is unauthenticated for password-less directory shares

http/data.go:29-48 d.Check is purely rule-based (deny/hidden globs); it does not reject control characters, so /<newline>FORGED passes Check by default

Proven fact: The ?files= query value is fully attacker-controlled and a percent-encoded %0a is decoded to a real newline before reaching the log call (raw.go:35).

Proven fact: gopath.Clean/filepath.Join/slashClean do not remove newline or carriage-return bytes, so the control byte survives into fname.

Proven fact: log.Printf at raw.go:181 uses %s (not %q/strconv.Quote), writing the newline verbatim into the Go standard-logger stream, splitting the record (CWE-117).

Proven fact: The error branch is trivially reachable: any nonexistent path that passes d.Check causes Fs.Stat to error and the log to fire.

Proven fact: publicDlHandler reaches rawDirHandler with no authentication when the share has an empty PasswordHash, making the injection pre-auth.

Unvalidated fact: The finding's evidence bullet asserting 'http/data.go prior hardening only quoted r.URL.Path (%q)' is INCORRECT: http/data.go:71 actually logs r.URL.Path with %s, not %q. This is immaterial to the verdict (the raw.go sinks are independently verified) and, if anything, shows data.go:71 is itself an analogous unescaped sink rather than a hardened contrast.

Unvalidated fact: Real-world impact depends on how the operator ingests logs (terminal view enabling ANSI/escape hazards vs. a line-oriented parser enabling forged-record spoofing) — a near-universal deployment fact, not a code gap.

http/raw.go:29,35,40-41 parseQueryFiles: ?files= value is split, url.QueryUnescape decodes %0a to a literal newline, then slashClean/gopath.Clean/filepath.Join preserve the control byte

http/raw.go:181 log.Printf("Failed to get files from %s: %v", fname, err) — fname is attacker-derived, emitted with %s (unescaped)

http/raw.go:112-120,152-155 getFiles: d.Check passes a crafted nonexistent path, Fs.Stat errors, driving the raw.go:181 error branch; raw.go:154 is a secondary %s sink over entry names

http/public.go:120-127 publicDlHandler->rawDirHandler and public.go:129-132 authenticateShareRequest returns (0,nil) when link.PasswordHash=="" — unauthenticated for password-less directory shares

http/data.go:29-48 d.Check is purely rule-based (deny/hidden globs) and does not reject control characters

Proven fact: log.Printf at raw.go:181 uses %s (not %q/strconv.Quote), writing the newline verbatim into the standard-logger stream (CWE-117).

Unvalidated fact: An evidence bullet in the source candidate claiming http/data.go only logs r.URL.Path with %q is incorrect: data.go:71 uses %s. Immaterial to this verdict (the raw.go sinks are independently verified); it merely shows data.go:71 is itself an analogous unescaped sink.

http/raw.go:181 - log.Printf("Failed to get files from %s: %v", fname, err) — %s on attacker-derived fname

http/raw.go:154 - log.Printf("Failed to get files from %s: %v", fPath, err) — secondary %s sink

http/raw.go:29,35,40-41 - parseQueryFiles: Query().Get + url.QueryUnescape decode %0a to newline; slashClean/gopath.Clean/filepath.Join preserve control bytes (Clean does not strip \n/\r)

http/raw.go:113,117 - getFiles: d.Check(path) passes on the crafted path, then Fs.Stat(path) fails for a nonexistent path, driving the error branch that logs fname

http/public.go:120-127 - publicDlHandler -> rawDirHandler makes the sink unauthenticated for password-less directory shares (authenticateShareRequest returns 0 when PasswordHash=="", public.go:130)

http/data.go prior hardening only quoted r.URL.Path (%q); these two raw.go sinks were left as %s — no existing finding/dismissal covers them for this component