fix(security): reject cross-origin requests in TrustedLocal#6
Merged
Conversation
TrustedLocal only checked that the TCP peer was loopback, so any web page open in the user's browser could fetch() the local /api and be treated as trusted -- a drive-by CSRF into RCE via /api/agent/exec and every other TrustedLocal-gated endpoint. Origin cannot be forged by a browser, so reject a request whose Origin host differs from its own Host; a request with no Origin header (curl, enx, scripts) is unaffected.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fetch()the local/apiand be treated as trusted, becauseTrustedLocalonly checked that the TCP peer was loopback and never checkedOrigin.Origin-vs-Hostcheck toTrustedLocal, the single choke point already used byAuthorized/Requirefor every/apiroute.Problem
server/middleware/dashboard.go'sTrustedLocalgrants full/apitrust to any loopback-peer request, with no check that it actually originated from enowX's own UI.Content-Type, a cross-site "simple" request (e.g.Content-Type: text/plain) is delivered by the browser without a CORS preflight even though the attacker page can't read the response — the side effect already happened.server/handlers/api_agent.go'sExec(arbitrary shell command) andFSWrite(arbitrary file write) sit behind this same gate — a single malicious tab open on the user's machine is drive-by RCE. The same hole reaches every otherTrustedLocal-gated endpoint (accounts, keys, mitm, integrations, plugins, market).Solution
originTrusted(r): ifOriginis absent, trust as before (curl, theenxCLI, and local scripts never send anOriginheader). If present, it must match the request's ownHost— a browser cannot forgeOrigin, so a mismatch reliably signals a cross-site request even when the TCP peer is genuinely loopback.r.Hostrather than a hardcoded127.0.0.1/localhostallow-list keeps this correct for every way a user legitimately reaches the app locally (127.0.0.1:PORT,localhost:PORT,[::1]:PORT, or a customENOWX_HOST).server/handlers/api_terminal.go, vianhooyr.io/websocket's defaultOriginPatternsbehavior) — this brings the plain-HTTP/apipath to parity with it instead of inventing a new policy.Changed files
server/middleware/dashboard.gooriginTrusted;TrustedLocalnow also requires it.server/middleware/dashboard_test.gonull, malformed, and forwarded+matching-Origin cases.Known limitation
OriginandHostfor consistency, not against a fixed allow-list, so a DNS-rebinding attack (attacker domain resolved to127.0.0.1) would present a matching-but-attacker-chosenOrigin/Hostpair and bypass this check. Flagging for visibility; considered out of scope for this fix.Test plan
go test ./server/middleware/... -run TestTrustedLocal -vpasses (9/9, including 1 pre-existing subtest)go build ./...andgo test ./...pass with no regressions (41 packages)/api/agent/execboth still execute; a mismatched-Origin request now gets401instead of executing