Pairing: global rate limit lets anyone lock out the code, and the code path hands out the permanent token #2

New Issue

benjamin · 2026-08-26T01:52:41+02:00

benjamin commented

2026-08-26 01:52:41 +02:00

Problem

Three weaknesses in handleHello() (ControlServer.kt:227-263) compound each other.

1. The rate limiter is global, not per client. codeAttempts is a single shared deque (ControlServer.kt:59). Worse, codeAttemptAllowed() is only reached when the message carries a non-empty code — a wrong token never consumes budget, but bogus code attempts do.

2. No Origin check. openWebSocket() never inspects Origin/Host, and NanoWSD does not either (confirmed: the class contains no Origin reference at all). WebSockets are not subject to same-origin policy, so any web page open in any browser on the LAN can connect to ws://<tv-ip>:8765/ from JavaScript.

3. The four-digit path yields the permanent token. The welcome payload always includes pairingToken (ControlServer.kt:254), regardless of whether authentication used the high-entropy token or the guessable code. The token never rotates (Pairing.kt).

Failure scenarios

Lockout: an attacker sends five bogus hello{code:…} per minute forever. The global budget is permanently starved, so the owner can never pair with the correct code. Trivially automated, needs only LAN access.
Brute force to permanent access: a malicious page left open in a browser tab tries codes at five per minute. 10,000 possible codes, expected success around 5,000 tries (~17 h), guaranteed within ~33 h. Success yields the permanent token, which never rotates — collapsing the intended two-tier trust model.

Suggested fix

Rate-limit per connection/remote address, and count every failed hello regardless of which field was wrong
Escalating backoff plus a cooldown after repeated failures, instead of a flat 5-per-60s window
Only include pairingToken in welcome when authentication used the token; code-based sessions get a session-scoped credential
Reject WebSocket upgrades whose Origin is present and not the server's own address

Found by multi-agent code review; verified against current ControlServer.kt.

## Problem Three weaknesses in `handleHello()` (`ControlServer.kt:227-263`) compound each other. **1. The rate limiter is global, not per client.** `codeAttempts` is a single shared deque (`ControlServer.kt:59`). Worse, `codeAttemptAllowed()` is only reached when the message carries a non-empty `code` — a wrong `token` never consumes budget, but bogus `code` attempts do. **2. No Origin check.** `openWebSocket()` never inspects `Origin`/`Host`, and NanoWSD does not either (confirmed: the class contains no `Origin` reference at all). WebSockets are not subject to same-origin policy, so any web page open in any browser on the LAN can connect to `ws://<tv-ip>:8765/` from JavaScript. **3. The four-digit path yields the permanent token.** The `welcome` payload always includes `pairingToken` (`ControlServer.kt:254`), regardless of whether authentication used the high-entropy token or the guessable code. The token never rotates (`Pairing.kt`). ## Failure scenarios - **Lockout:** an attacker sends five bogus `hello{code:…}` per minute forever. The global budget is permanently starved, so the owner can never pair with the correct code. Trivially automated, needs only LAN access. - **Brute force to permanent access:** a malicious page left open in a browser tab tries codes at five per minute. 10,000 possible codes, expected success around 5,000 tries (~17 h), guaranteed within ~33 h. Success yields the permanent token, which never rotates — collapsing the intended two-tier trust model. ## Suggested fix - Rate-limit per connection/remote address, and count every failed `hello` regardless of which field was wrong - Escalating backoff plus a cooldown after repeated failures, instead of a flat 5-per-60s window - Only include `pairingToken` in `welcome` when authentication used the token; code-based sessions get a session-scoped credential - Reject WebSocket upgrades whose `Origin` is present and not the server's own address Found by multi-agent code review; verified against current `ControlServer.kt`.

benjamin referenced this issue from a commit

2026-08-26 02:14:49 +02:00

Resolve the full review backlog (issues #1-#13)

benjamin commented

2026-08-26 02:15:57 +02:00

Fixed in dd612fc, shipped in v0.9.0. All three weaknesses are addressed:

Rate limiting is per remote address and counts every failed hello, whether it carried a token or a code (attemptAllowed(remoteAddress) runs before any credential is inspected). One hostile client can no longer starve the owner's budget; the tracking map is pruned so it cannot grow without bound.
Origin is checked on the upgrade: serve() rejects any request whose Origin header is present and does not match the server's own host, with 403. A page on another origin can still open a socket at the TCP level but never gets past the HTTP upgrade.
The code path no longer yields the QR token. A code-authenticated client receives a freshly generated session token (Pairing.newSessionToken), stored server-side in a capped list and revocable independently. The QR token is only echoed back to clients that already presented it.

Constant-time comparison (MessageDigest.isEqual) is used for both token and code, and "Kopplung zurücksetzen" in the settings invalidates everything at once (see #3).

Fixed in dd612fc, shipped in v0.9.0. All three weaknesses are addressed: 1. **Rate limiting is per remote address** and counts *every* failed `hello`, whether it carried a token or a code (`attemptAllowed(remoteAddress)` runs before any credential is inspected). One hostile client can no longer starve the owner's budget; the tracking map is pruned so it cannot grow without bound. 2. **Origin is checked** on the upgrade: `serve()` rejects any request whose `Origin` header is present and does not match the server's own host, with 403. A page on another origin can still open a socket at the TCP level but never gets past the HTTP upgrade. 3. **The code path no longer yields the QR token.** A code-authenticated client receives a freshly generated session token (`Pairing.newSessionToken`), stored server-side in a capped list and revocable independently. The QR token is only echoed back to clients that already presented it. Constant-time comparison (`MessageDigest.isEqual`) is used for both token and code, and "Kopplung zurücksetzen" in the settings invalidates everything at once (see #3).

benjamin closed this issue

2026-08-26 02:15:58 +02:00

benjamin referenced this issue

2026-08-26 02:16:14 +02:00

Pairing token travels in cleartext, lands in cloud backups and never rotates #3

benjamin referenced this issue

2026-08-26 02:16:29 +02:00

set_playlist: unrestricted URL fetch (SSRF) with unbounded response buffering #4

benjamin referenced this issue from a commit

2026-08-26 02:35:25 +02:00

Compose for TV UI, generic + Dispatcharr sources, device-flow login, favorites, stream profiles, richer remote

benjamin referenced this issue from a commit