[Security] No maximum frame size — 4 GiB OOM via crafted message

[iRonin]

Summary

The framing protocol reads a 4-byte big-endian length prefix and then waits for that many bytes of payload, with no upper bound on the prefix value. A peer can announce a 4 GiB frame and the broker will allocate up to that much memory in the per-connection accumulation buffer. The minimum cost to crash the broker (and disrupt every connected pi session) is one TCP-style write.

Affected code

• File: broker/framing.ts:25-58 (verified at commit 5caa4aa)

return (data: Buffer) => {
  buffer = Buffer.concat([buffer, data]);

  while (buffer.length >= 4) {
    const length = buffer.readUInt32BE(0);

    if (buffer.length < 4 + length) {
      break;
    }

    const payload = buffer.subarray(4, 4 + length);
    buffer = buffer.subarray(4 + length);
    // ...
  }
};

There is no if (length > MAX_FRAME) onError(...) after readUInt32BE. The only constraint on length is the type (UInt32BE → max 0xFFFFFFFF = ~4.29 GiB).

writeMessage is the producer side and has no cap either, but in practice that's pi sending its own messages so it's bounded by sanity — the inbound side is the attack surface.

Reproduction / attack scenario

1. Attacker connects to ~/.pi/agent/intercom/broker.sock (no auth needed — see related issues).
2. Attacker writes \xFF\xFF\xFF\xFF and then either:
   • Slow OOM: streams arbitrary bytes at network speed. The broker's Buffer.concat([buffer, data]) re-allocates and copies the accumulated buffer on every chunk, so memory pressure is roughly the total bytes received, peaking near 4 GiB. On a typical dev laptop that's an OOM kill long before reaching 4 GiB.
   • Fast wedge: never sends the body. The connection stays open with buffer holding 4 bytes forever. Combine with N parallel connections and the accept queue itself becomes a resource problem, though the bigger issue is just that the broker has no idea anything is wrong.
3. When the broker is OOM-killed, every connected pi session loses its intercom connection mid-turn. The clients reconnect (per README), but the auto-spawn logic re-launches the broker, and the attacker reconnects too. Repeat.

Buffer.concat allocating new buffers on every chunk also means a 1 GiB legitimate-looking frame from a malicious peer triggers a 1 GiB allocation in V8's old-space, which on Node's default heap (~1.7 GiB on a 64-bit build, less on 32-bit) is itself an OOM.

Impact

HIGH. Single-write DoS of the broker, which cascades to every connected pi session. Persistent: the broker auto-respawns and the attacker is also auto-reconnectable, so the user has no good "wait it out" recovery — they have to find and kill the attacker process. Combined with the no-rate-limit issue (filed separately), this is a one-line denial of every multi-session pi workflow on the box.

Suggested mitigation

Cap the frame size at something reasonable for the protocol (1 MiB is generous — current real frames are kilobytes max even with attachments) and reject anything larger:

const MAX_FRAME_BYTES = 1 * 1024 * 1024; // 1 MiB

while (buffer.length >= 4) {
  const length = buffer.readUInt32BE(0);

  if (length > MAX_FRAME_BYTES) {
    onError(new Error(`Intercom frame too large: ${length} bytes (max ${MAX_FRAME_BYTES})`));
    return;
  }

  if (buffer.length < 4 + length) {
    break;
  }
  // ...
}

The onError path in broker.ts:handleConnection already calls socket.destroy(error), so a single guard here both bounds memory and closes the offending peer immediately.

Two small follow-ups worth considering at the same time:

• Cap the per-connection accumulation buffer separately (e.g. buffer.length > MAX_FRAME_BYTES + 4 should also be a hard error, in case a peer streams just under the limit but never completes).
• Replace Buffer.concat([buffer, data]) with a smarter reader (e.g. a Readable consuming pattern, or a list of chunks reassembled only when a complete frame is available) to avoid the O(n²) copy cost on legitimate large frames near the cap.

Environment

• Repo: nicobailon/pi-intercom @ 5caa4aa
• Reported by: external security review (read-only audit)