SiYuan: A parser differential between the WebSocket keepalive exemption and the session quarantine allows unauthenticated access to the broadcast event stream

### Summary Two checks on the same WebSocket request URI parse it differently. The authentication exemption for keepalive sessions matches raw substrings, while the quarantine introduced to contain those sessions reads a parsed query parameter. Because Go's `url.Values.Get` returns only the first value for a repeated key, a single URI can satisfy the exemption and still resolve to a non-auth session identity. The result is that an unauthenticated remote caller connects to the kernel WebSocket with a valid access auth code configured, is placed in the general broadcast pool rather than the quarantined pool, and receives the live event stream. ### Details **The exemption matches substrings.** `kernel/server/serve.go:1222` on the development branch, `:1074` on master: ```go authOk = strings.Contains(s.Request.RequestURI, "/ws?app=siyuan") && strings.Contains(s.Request.RequestURI, "&id=auth&type=auth") ``` Neither operand is parsed. Any URI containing both literals is exempted, regardless of what the query actually resolves to. **The quarantine parses.** Commit `1e370e373` added the containment for exempted sessions at `kernel/util/websocket.go:171`: ```go id = session.Request.URL.Query().Get("id") return "auth" == id ``` `url.Values.Get` returns the first value for a repeated key. **The differential.** For the URI: ``` /ws?app=siyuan&id=main&type=main&id=auth&type=auth ``` - It contains `/ws?app=siyuan` and `&id=auth&type=auth`, so `authOk` is true. No cookie, no JWT and no access auth code are required. - `Query().Get("id")` returns `main`, so `IsAuthSession` is false. `AddPushChan` files the session in `sessions`, the broadcast pool, rather than in `authSessions`. - `Query().Get("type")` returns `main`, so the session matches `BroadcastByType("main", ...)`. - Without a valid `X-Auth-Token` the publish flag is never set, so `isPublishSession()` does not exclude it either. Every guard that should have caught the session keys off a different parse than the one that admitted it. **No origin validation.** `CheckOrigin` does not appear on the `/ws` path, so the cross-origin vector described in the parent advisory also remains available. **Distinction from the existing broadcast-pool advisory.** GHSA-mw8r-mw84-88v2 concerns the publish port, where an anonymous session is expected to connect and the defect is that broadcast content is not gated for it afterwards. This report concerns the kernel port with an access auth code configured, where the session should never be admitted in the first place. The leaked data overlaps because both end in the same broadcast pool, but the mechanisms and the affected deployments are different, and gating publish sessions does not close this path. ### Proof of Concept Kernel port, `--accessAuthCode` set. Three WebSocket connections, none carrying credentials of any kind. | Socket | Query | Result | |---|---|---| | A, baseline | `?app=siyuan&id=main&type=main` | Rejected, closed 1002, 0 frames | | B, control | `?app=siyuan&id=auth&type=auth` | Connects, 0 frames | | C, attack | `?app=siyuan&id=main&type=main&id=auth&type=auth` | Connects, 19 to 30 frames across two runs | Socket A establishes that the access auth code is enforced. Socket B establishes that the existing quarantine works exactly as intended, admitting the keepalive session but delivering nothing to it. Socket C differs from B only in the duplicated parameters, and receives the event stream. Frames observed on socket C included `savedoc` (`"action":"create","data":{"Root":{"ID": ...`), `transactions` (`"action":"updateAttrs"` carrying document titles), `filetreeSortChanged` (the notebook's complete ordered `childIDs`), `create` (a notebook object including its `name`), and `backgroundtask`. A canary string placed inside a document body did not appear in any frame, on a full-text search of all captured frames. ### Impact An unauthenticated remote caller obtains the kernel's live event stream on a deployment protected by an access auth code, with no user interaction and no prior knowledge of the instance. Precisely what is disclosed: document identifiers and titles, notebook identifiers and names, file paths, the complete ordered child structure of notebooks, and the full stream of create, update and delete operations as they occur. Document body content is not present in the observed frames. That set corresponds to the disclosure claimed by GHSA-xp2m-98x8-rpj6, which means the remediation for that advisory is bypassable rather than incomplete in scope. The absence of origin validation on the endpoint additionally preserves the cross-origin reachability described there. Confidentiality only. No frames observed permitted writes. ### Suggested fix Make both checks agree, and stop re-parsing attacker-controlled input after admission. 1. In the exemption, parse the query rather than matching substrings, and require `id` and `type` each to equal `auth` exactly. 2. Reject requests carrying duplicated query keys on this endpoint, so no URI can present two identities. 3. Record the exemption as a flag on the session at connect time, and have `IsAuthSession` read that flag rather than re-parsing the request. A session admitted under the keepalive exemption is then quarantined by construction, independent of how its URI is read later. Adding origin validation to `/ws` would additionally close the cross-origin vector.