Skip to content

[Security] ClusterClient's socket-data path has no try/catch around FrameDecoder — one malformed frame from a contact-point crashes the client process #587

Description

@pathosDev

Component: src/cluster/ClusterClient.ts
Severity (assessment): LOW
CWE: CWE-248

ClusterClient.onData calls this.decoder.push(chunk) unguarded. FrameDecoder.push throws on an oversized length-prefix or invalid JSON, and the call sits directly inside the runtime's socket data callback, so the throw escapes into EventEmitter.emit (Node) / the Bun socket handler and becomes an uncaught exception. TcpTransport.onData wraps the identical call in a try/catch for exactly this reason; the client was never given the same treatment.

Exploit walkthrough

A compromised or malicious cluster contact-point — or, because tls is optional and unset by default (ClusterClient.ts:120, this.tls = resolvedOptions.tls ?? null), a network attacker injecting a segment into the plaintext TCP stream — sends five bytes: FF FF FF FF followed by anything. push computes len = 4294967295 > maxFrameBytes and throws wire frame claims length … connection terminated to prevent OOM/DoS. The exception unwinds through raw.on('data', …) in NodeTcpBackend.connect with no handler, so the whole application process hosting the ClusterClient (typically a REST frontend or a batch job, per the file header) terminates. Malformed JSON in a well-sized frame does the same via Invalid wire frame JSON. Note the same method already contains a try/catch one branch lower, for reply handling — so the omission is at the decode step only.

Evidence — src/cluster/ClusterClient.ts:277

src/cluster/ClusterClient.ts:272-278:
```
  private onData(
    sock: TcpSocketLike,
    chunk: Uint8Array,
    onHelloAcknowledgment: (peer: NodeAddress) => void,
  ): void {
    const frames = this.decoder.push(chunk);
    for (const frame of frames) {
```
The transport-side counterpart, src/cluster/Transport.ts:206-218, shows the intended handling:
```
    try {
      frames = connection.decoder.push(chunk);
    } catch (err) {
      // Frame-decoder rejected the input (oversized length-prefix,
      // malformed JSON).  Drop the connection rather than letting the
      // error propagate up the runtime's socket-data callback.
```
src/runtime/tcp/NodeTcpBackend.ts:70  the unprotected call site:
```
    raw.on('data', (chunk: Buffer) => options.handlers.onData(sock, toUint8(chunk)));
```

Why the existing guard does not cover it

I checked for a guard at each layer: (1) inside ClusterClient.onData — the only try/catch is around this.handleReply(...) (lines 291-296), added for a different bug (one bad reply abandoning the rest of a batch), and it sits after push has already thrown; (2) inside the backends — NodeTcpBackend/BunTcpBackend forward onData straight to the handler with no wrapper; (3) a process-level uncaughtException trap anywhere in src/ — none; (4) tests — tests/multi-node/cluster-client.test.ts covers ask/send/failover/unknown-path only, and tests/integration/in-process/cluster/ClusterClient.askId.test.ts covers ask-id entropy; no malformed-frame case exists. Distinct from the tracked #144 (TLS config validation at construction) — this fires whether or not TLS is configured.

Suggested fix

Mirror TcpTransport.onData: wrap this.decoder.push(chunk) in a try/catch, log the error, tear the socket down (this.socket?.end()) and route through onSocketClose() so pending asks reject with a real reason instead of the process dying. A fresh FrameDecoder is already created there, so recovery is clean.

Verification status

Found in the whole-framework security audit of 2026-08-01 (v0.12.0), then adjudicated by an independent verifier instructed to refute it.

Verifier note

ClusterClient.ts:277 const frames = this.decoder.push(chunk); is unguarded — the only try/catch in the method is at lines 291-296 around handleReply, after push has already thrown. push does throw on both cited inputs (Protocol.ts:199-202 oversized length-prefix, Protocol.ts:211 invalid JSON), and the handler is installed at ClusterClient.ts:237-244 inside backend.connect, whose Node implementation forwards it raw: NodeTcpBackend.ts:70 raw.on('data', (chunk) => options.handlers.onData(sock, toUint8(chunk))) with no wrapper (BunTcpBackend.ts:29/60 likewise). TcpTransport.onData wraps the identical call (Transport.ts:207-218) precisely to stop this, so the asymmetry is real.

Correction applied: Impact is narrower than 'medium' implies. It is an availability bug in the client process only, and the trigger requires either a malicious/compromised contact-point (a node the client was explicitly configured to talk to) or an active MitM on a plaintext link — not an anonymous internet attacker. Note DenoTcpBackend.ts:114-121 already catches a throwing onData and routes it to onError/onClose, so the crash is Node/Bun-only. The most likely real-world trigger is benign: an ask reply body larger than the client's default 16 MiB cap kills the process instead of rejecting the ask. Fix as proposed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: lowNice-to-have / niche / demand-drivensecuritySecurity-relevant — see severity label for impact tierseverity: lowMinor / informational / mitigated-by-design

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions