Skip to content

[Security] GrpcClientActor delivers the bidi stream-id handshake in-band as a stream-data chunk and bidiSend/bidiClose resolve any id against a client-wide map with no ownership check #788

Description

@pathosDev

Component: src/io/broker/GrpcClientActor.ts
Severity (assessment): INFORMATIONAL
CWE: CWE-863 (Incorrect Authorization) / CWE-436 (Interpretation Conflict)

After bidiStart, the generated stream id is delivered to the target as an ordinary stream-data frame whose chunk is { __streamId } — structurally indistinguishable from a server-sent message that happens to carry a __streamId field. The documented way to obtain the handle is to read that field off a chunk. Separately, bidiSend/bidiClose look the id up in a map shared by every stream the actor owns, with no check that the sender started that stream.

Exploit walkthrough

Preconditions: one GrpcClientActor shared by two or more application actors — the documented deployment ("One client instance per service", GrpcClientActor.ts:72-74) — with at least one bidi method whose response message type has, or can be extended to have, a field named __streamId (__streamId is a legal protobuf identifier; with keepCase: true at :121 the wire name is preserved verbatim).

  1. Actor A opens stream 1 and actor B opens stream 2 against the same client.
  2. A malicious or compromised server sends actor A a normal data chunk deserialising to { __streamId: 2 }.
  3. Actor A, following the documented pattern, reads chunk.__streamId — it has no way to tell the framework's synthetic handshake frame from server payload, since both arrive as { kind: 'stream-data', streamId, chunk } — and takes 2 as its handle.
  4. Every subsequent bidiSend from A is written into actor B's stream, because onBidiSend resolves the id globally and never checks that A owns it; bidiClose likewise tears down B's stream. Requests cross tenant/session boundaries inside the client, and B's stream is closed by a party that never opened it.
    Uncertainty is honest here: it needs the app to follow the documented chunk.__streamId read and needs the server's message type to carry that field. A robust app that tracks its own stream id positionally is unaffected — but the docs teach the vulnerable pattern.

Evidence — src/io/broker/GrpcClientActor.ts

src/io/broker/GrpcClientActor.ts:249-250:

    // Send the streamId back so the caller can address future bidiSend/Close.
    op.target.tell({ kind: 'stream-data', target: op.target, streamId, chunk: { __streamId: streamId } } as never);

and the server's own chunks arrive on the identical envelope — :238-240:

    call.on('data', (chunk: unknown) => {
      op.target.tell({ kind: 'stream-data', target: op.target, streamId, chunk } as never);
    });

The documented consumption pattern reads the field straight off a chunk — docs/src/content/docs/io/grpc.mdx:236-242:

// Inside the collector, after receiving the streamId hint:
const streamId = (message.chunk as { __streamId: number }).__streamId;

client.tell({ kind: 'bidiSend', streamId, chunk: { text: 'hello' } });

The lookup is a bare map read against a map shared by all streams (:86, private readonly bidiStreams = new Map<...>()) — src/io/broker/GrpcClientActor.ts:173-184:

  private onBidiSend(command: BidiSendCommand): void {
    const stream = this.bidiStreams.get(command.streamId);
    if (stream) stream.call.write(command.chunk);
  }

  private onBidiClose(command: BidiCloseCommand): void {
    const stream = this.bidiStreams.get(command.streamId);
    if (stream) {
      try { stream.call.end(); } catch { /* ignore */ }
      this.bidiStreams.delete(command.streamId);
    }
  }

The map entry records the owning target (:237, { call, target: op.target }) but neither handler ever reads it. Ids are a plain sequential counter (:85, private nextStreamId = 1;), so they are trivially guessable.

Why the existing guard does not cover it

The map entry stores target alongside call (:237), so the information needed for an ownership check is already present and simply unused. The comment at :167-171 explains that an unknown id is a deliberate no-op ("the stream is already gone") — correct for a stale id, but it is silent about a live id owned by someone else, which is the case that matters. nextStreamId is monotonic so ids are never recycled, which rules out stale-id confusion but does nothing for cross-actor confusion.

Suggested fix

Take the handshake out of band: add a dedicated { kind: 'stream-started'; streamId } variant to GrpcInbound so the id can never be confused with server payload, and update the docs to read it from that message rather than from chunk.__streamId. Then enforce ownership in onBidiSend/onBidiClose — compare stream.target with the command's originator (carry it on BidiSendCommand/BidiCloseCommand, or use context.sender) and ignore/log a mismatch.

Verification status

Found in the second, independent whole-framework security re-audit of 2026-08-02 (v0.12.0) — a fresh pass run without reference to the first wave's findings, then triaged against the existing tracker and adjudicated by verifiers instructed to refute it.

Verifier note

Code facts check out. GrpcClientActor.ts:250 emits { kind: 'stream-data', target, streamId, chunk: { __streamId: streamId } }, the same envelope shape used for server chunks at :239. onBidiSend/onBidiClose (:173-184) resolve command.streamId against this.bidiStreams (:86), a map shared by every stream this client owns, and never consult the target the entry records at :237. Ids come from a plain sequential counter (nextStreamId = 1, :85). The documented consumption pattern is as quoted — docs/src/content/docs/io/grpc.mdx:214-221 (and the German mirror at de/io/grpc.mdx:218-224) instruct the reader to do (message.chunk as { __streamId: number }).__streamId, and tests/integration/brokers/grpc/scenarios/03-bidi.ts:32-40 does the same, even filtering real chunks with __streamId === undefined at :60.

But the finding's framing is wrong on the point that carries it. The stream-data envelope already carries the framework-generated streamId as a top-level field (StreamDataMessage, :16-21), set identically for the handshake (:250) and for every server chunk (:239). An application that reads message.streamId gets the right id unconditionally and cannot be steered by server payload — so the out-of-band channel the fix proposes largely exists already, and the __streamId field inside the chunk is pure redundancy. The docs' own server-stream paragraph even says the frames carry streamId; only the bidi paragraph reaches into the chunk. That reduces the confusion half to a documentation bug with an existing safe alternative. The remaining code defect — onBidiSend/onBidiClose accepting any id from any sender with the owning target sitting unused one field away — is genuine, but reaching it requires holding the client's ActorRef, which in-process grants the full command surface anyway. Corrected LOW → INFORMATIONAL and reframed: fix the docs (EN + DE) to read message.streamId, drop or supplement the in-band __streamId, and add the ownership check that :237 already has the data for.

Correction applied: The central claim — that the application "has no way to tell the framework's synthetic handshake frame from server payload" — is wrong. The StreamDataMessage envelope carries the authoritative streamId as a top-level field (type at GrpcClientActor.ts:16-21; set from the framework's own counter at :239 for server chunks and at :250 for the handshake). Reading message.streamId is always correct and cannot be influenced by the server, so an out-of-band channel already exists — the docs simply teach the wrong one. The confusion is therefore a documentation defect (docs/src/content/docs/io/grpc.mdx:214-221 and the German mirror de/io/grpc.mdx:218-224 tell the reader to read chunk.__streamId, and tests/integration/brokers/brokers/grpc/scenarios/03-bidi.ts follows suit), not a structural impossibility. The second half — no ownership check in onBidiSend/onBidiClose — is real but its precondition is possession of the client actor's ActorRef, which inside a single process is not a trust boundary. Severity corrected LOW → INFORMATIONAL.

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