mirror of
https://github.com/bitwarden/clients.git
synced 2026-07-21 21:17:06 +08:00
a74e433542
* fix(cli): harden bw serve origin-protection against DNS rebinding Closes PM-36600 / VULN-536. HackerOne report 3684172. The prior origin-protection middleware only checked for the presence of an Origin header, which DNS-rebinding attacks bypass: the browser rebinds an attacker-controlled hostname to 127.0.0.1, then issues same-origin GETs that carry no Origin header (per the Fetch spec) but carry the attacker's hostname in the Host header. Any malicious page the victim visits could silently read the unlocked vault. The fix introduces two middleware layers, guarding every route method-agnostically: Layer 1 (new) — Host-header allowlist. Requests are rejected unless their Host header matches localhost:PORT, 127.0.0.1:PORT, [::1]:PORT, or the user-configured --hostname value at the configured port. This closes the DNS-rebinding vector: a rebound request carries the attacker's hostname in Host, not a loopback identifier. The allowlist construction is extracted into an exported helper `buildServeAllowedHosts(hostname, port)` so it is unit-testable. Layer 2 (retained, behavior unchanged) — Origin check. Rewritten from `!= undefined` to `!== undefined` for stylistic consistency with Layer 1. This is a runtime no-op: Node's IncomingHttpHeaders types `ctx.headers.origin` as `string | string[] | undefined` and never produces JS null, so an HTTP `Origin: null` header arrives as the string "null" and was already rejected by the prior check. Layer 2 is retained as defense-in-depth for classical cross-origin fetches: any defined Origin header value is rejected. A new spec encodes the four-probe regression contract: Probe 1: cross-origin attack (Origin: https://evil.example) -> 403 Probe 2: legitimate localhost (Host: 127.0.0.1, no Origin) -> 200 Probe 3: DNS-rebind (Host: evil.example, no Origin) -> 403 [THE FIX] Probe 4: empty Origin (forward-regression guard) -> 403 Three additional unit tests cover the `buildServeAllowedHosts` helper. No new npm dependencies introduced. * fix(cli): refine Host allowlist for --hostname all and default ports Two allowlist gaps were found in automated review of PR #20881 and addressed here. 1. --hostname all: `buildServeAllowedHosts` produced a set containing literal "all:PORT", so LAN clients sending `Host: 192.168.1.5:PORT` received a 403. The operator already opted into multi-interface binding by passing `--hostname all`; enumerating live IPs at runtime is fragile. Fix: when `hostname === "all"`, pass `null` for `allowedHosts`, disabling Layer 1 while keeping Layer 2 (Origin check) active. A startup log entry makes the posture visible. 2. RFC 7230 §5.4 default-port omission: HTTP clients omit the default port from `Host` when it matches the scheme default (80 for http, 443 for https). `bw serve --hostname bwapi.mydomain.com --port 80` produced a 403 because `Host: bwapi.mydomain.com` was not in the allowlist. Fix: when port is 80 or 443, also push bare-host variants (`localhost`, `127.0.0.1`, `[::1]`, configured hostname) onto the set. Non-default ports are intentionally excluded — a missing port against a non-default port is a genuine client misconfiguration. `buildOriginProtectionMiddleware`'s `allowedHosts` parameter widens to `ReadonlySet<string> | null` to express "Layer 1 disabled." 7 new test cases added (12 passing total, up from 5): covers null allowedHosts behavior and bare-host entries at ports 80, 443, and 8087 (negative case). Refs: PM-36600 * review: coerce port to a number |
||
|---|---|---|
| .. | ||
| completion.command.ts | ||
| download.command.ts | ||
| edit.command.spec.ts | ||
| edit.command.ts | ||
| encode.command.ts | ||
| get.command.ts | ||
| list.command.ts | ||
| restore.command.ts | ||
| serve.command.spec.ts | ||
| serve.command.ts | ||
| status.command.ts | ||
| update.command.ts | ||