WebSocket /api/command upgrades and reads unbounded/undeadlined before the exec-authorization check

Status

Repository / Component

Plain-English Description

Any logged-in user can open the command WebSocket even without permission to run commands, and the server will hold the connection open forever or accept an unlimited-size message before it checks their permission. Repeating this can tie up server memory and connections until the service degrades.

Description of the Underlying Issue

The /api/command route (http/http.go:85) is registered unconditionally with no EnableExec gate at routing time. commandsHandler (http/commands.go:41-46) is wrapped only by withUser, and calls upgrader.Upgrade before any exec-authorization check. It then loops on conn.ReadMessage() (lines 50-61) with no SetReadLimit and no SetReadDeadline. The EnableExec/Perm.Execute check at line 64 runs only after a non-empty message is received, so it never fires for a client that completes the handshake and stays silent, or that streams one huge message. gorilla/websocket defaults readLimit to 0 (unbounded) and ReadMessage buffers the whole message via ioutil.ReadAll, and the server (cmd/root.go:246-249) sets only ReadHeaderTimeout, which does not apply after the WS hijack.

Potential Attack

An attacker authenticates as any user (exec permission not required, since the auth cookie is accepted on the GET handshake via withUser) and opens GET /api/command with the Upgrade handshake. Variant A: complete the handshake and never send a message — the server goroutine blocks in ReadMessage with no deadline, indefinitely holding a connection and file descriptor; repeat to pin many. Variant B: send one very large fragmented/streamed message — ReadMessage's ReadAll buffers it entirely into memory because readLimit is 0.

Outcomes of Potential Attack

Sustained repetition exhausts server goroutines, file descriptors/connections, and (variant B) memory, degrading or denying service for all users. No command execution or data disclosure results; the impact is availability.

Affected Scope

WS GET /api/command handler (http/commands.go); any authenticated user, including users without EnableExec/Perm.Execute

Suggested Fix (plain english)

Check the run-command permission before upgrading the connection, and put size and idle-time limits on the socket right after it opens.

Suggested Fix (detailed)

Move the EnableExec/Perm.Execute check before upgrader.Upgrade so unauthorized users never establish a socket. Immediately after Upgrade call conn.SetReadLimit(reasonableMax) and conn.SetReadDeadline(now+timeout), refreshing the deadline on each read. Additionally set http.Server ReadTimeout/IdleTimeout and consider capping concurrent command sockets per user. Verify: a silent client is dropped after the deadline, and an oversized message is rejected rather than buffered. Call conn.SetReadLimit(reasonableMax) and conn.SetReadDeadline(now+timeout) immediately after Upgrade (and refresh the deadline per read). Move the EnableExec/Perm.Execute check before upgrader.Upgrade so unauthorized users never establish a socket. Consider setting http.Server ReadTimeout/IdleTimeout and capping concurrent command sockets per user.

Validation

Independently confirmed in current source and gorilla/websocket@v1.5.3 dependency source. The structural defect (upgrade+unbounded/undeadlined read before the exec-authz gate, reachable by any authenticated user, with no server idle/read timeout) is complete; the resulting exhaustion magnitude is gated on host limits and any fronting-proxy WS caps, so possibly_valid with that precondition named.

first_opinion

Full Evidence

http/http.go:85 - api.PathPrefix("/command").Handler(monkey(commandsHandler,...)).Methods("GET") registered unconditionally; no EnableExec gate at routing

http/commands.go:41-46 - commandsHandler = withUser(...); upgrader.Upgrade(w,r,nil) runs before any exec-authz check

http/commands.go:50-61 - for{ conn.ReadMessage() } with no SetReadLimit and no SetReadDeadline; loops until a non-empty trimmed message

http/commands.go:64 - EnableExec/Perm.Execute deny path runs AFTER a message is received, so it never fires for a silent client

http/auth.go:57-61,83-108 - withUser requires only a valid HS256 JWT; the 'auth' cookie is accepted on GET, so the WS handshake authenticates via ambient cookie

cmd/root.go:246-249 - http.Server sets only ReadHeaderTimeout (60s); no ReadTimeout/IdleTimeout, and header timeout does not apply after the WS hijack

gorilla/websocket@v1.5.3 conn.go:924 - readLength limit enforced only when c.readLimit>0; default readLimit is 0 (unbounded)

gorilla/websocket@v1.5.3 conn.go:1091-1098 - ReadMessage() buffers the whole message via ioutil.ReadAll(r)

Proven fact: The handler is reachable by authentication alone; the exec-authorization gate is positioned after the first ReadMessage, so it does not protect the upgrade or the blocking read.

Proven fact: No SetReadLimit is ever called (verified across http/*.go), so gorilla's per-message buffer (ioutil.ReadAll) is unbounded for a single message.

Proven fact: No SetReadDeadline is set and the server has no ReadTimeout/IdleTimeout, so a client that completes the handshake and stays silent pins one goroutine + one connection/FD indefinitely in conn.ReadMessage().

Unvalidated fact: Whether a given deployment fronts filebrowser with a reverse proxy that imposes a WebSocket idle timeout or message-size cap (would mitigate the silent-hold and oversized-message vectors) — a deployment fact not settleable from source.

Unvalidated fact: The absolute request volume needed to reach FD/goroutine/memory exhaustion depends on the host's ulimit and available memory (runtime facts).

http/http.go:85 — /command handler registered unconditionally on GET; no EnableExec gate at routing

http/commands.go:41-46 — commandsHandler = withUser(...); upgrader.Upgrade runs before any exec-authz check

http/commands.go:50-61 — for{ conn.ReadMessage() } with no SetReadLimit and no SetReadDeadline

http/commands.go:64 — EnableExec/Perm.Execute deny path runs after a message is received, so never fires for a silent client

http/auth.go:57-61,83-108 — withUser requires only a valid HS256 JWT; the auth cookie is accepted on GET

cmd/root.go:246-249 — http.Server sets only ReadHeaderTimeout (60s); no ReadTimeout/IdleTimeout, and header timeout does not apply after the WS hijack

gorilla/websocket@v1.5.3 conn.go:924 — readLength limit enforced only when readLimit>0; default 0 (unbounded)

gorilla/websocket@v1.5.3 conn.go:1091-1098 — ReadMessage buffers the whole message via ioutil.ReadAll

Proven fact: No SetReadLimit is ever called (verified across http/*.go), so gorilla's per-message buffer is unbounded for a single message.

Proven fact: No SetReadDeadline is set and the server has no ReadTimeout/IdleTimeout, so a client that completes the handshake and stays silent pins one goroutine + one connection/FD indefinitely.

Unvalidated fact: Whether a given deployment fronts filebrowser with a reverse proxy that imposes a WebSocket idle timeout or message-size cap — a deployment fact not settleable from source.

Unvalidated fact: The absolute request volume needed to reach FD/goroutine/memory exhaustion depends on the host's ulimit and available memory.

http/http.go:85 - api.PathPrefix("/command").Handler(monkey(commandsHandler, "/api/command")).Methods("GET") registered unconditionally (no EnableExec gate at routing)

http/commands.go:41 - commandsHandler = withUser(...) (authentication is the only gate to reach the handler body)

http/commands.go:42 - upgrader.Upgrade(w, r, nil) happens before any exec authorization

http/commands.go:50-61 - read loop: conn.ReadMessage() with no SetReadLimit and no SetReadDeadline; loops until a non-empty trimmed message

http/commands.go:64-70 - EnableExec/Perm.Execute deny path runs AFTER a message is received, so it never fires for silent clients

http/auth.go:57-61 - extractor accepts the 'auth' cookie for GET requests, enabling the WS handshake to authenticate via ambient cookie

cmd/root.go:246-249 - http.Server sets only ReadHeaderTimeout (60s); no ReadTimeout/IdleTimeout, and header timeout does not apply post-hijack

gorilla/websocket@v1.5.3 conn.go:1091-1099 - ReadMessage() uses ioutil.ReadAll to buffer the entire message

gorilla/websocket@v1.5.3 conn.go:917-924 - readLength capped only when readLimit>0; default readLimit is 0 (unbounded)

gorilla/websocket@v1.5.3 conn.go:1101-1114 - SetReadDeadline/SetReadLimit exist but filebrowser never calls them (grep of http/*.go finds no SetReadLimit/SetReadDeadline)