Skip to content

fix(sdk-node): cap wire-controlled frame lengths before allocation (PILOT-103)#3

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-103-20260528-064502
Open

fix(sdk-node): cap wire-controlled frame lengths before allocation (PILOT-103)#3
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-103-20260528-064502

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What failed

src/client.ts reads data-exchange ACK payloads (conn.read(ackLen)) and event-stream topic/payload frames (conn.read(topicLen), conn.read(payloadLen)) using wire-controlled lengths parsed from remote bytes without any size cap. A malicious or buggy peer can advertise a 4 GiB length and force the SDK to attempt a single huge allocation → memory exhaustion / OOM kill DoS on the host process.

What was changed

Added MAX_PAYLOAD_SIZE = 1_048_576 (1 MiB) and exported MAX_TOPIC_SIZE = 4_096 (4 KiB) constants. Before each conn.read(N) where N comes from the wire:

  • sendMessage / sendFile ACK: if ackLen > 1 MiB, skip the ACK read and return the basic result (same as a failed/incomplete ACK read today — already handled by the caller)
  • readEventFrame: if topicLen > 4 KiB or payloadLen > 1 MiB, return null (treated identically to an incomplete read — benign)

Verification

  • tsc build: ✅
  • All 173 existing tests pass ✅
  • 1 new cap export test added ✅

Closes PILOT-103

…ILOT-103)

SDK read data-exchange ACK payloads and event-stream topic/payload
frames using wire-controlled lengths (readUInt32BE/readUInt16BE on
remote bytes) without any size cap. A malicious peer could advertise
a 4 GiB length and force the SDK to attempt a single huge allocation,
causing memory exhaustion and OOM kill on the host process.

Add MAX_PAYLOAD_SIZE (1 MiB) and MAX_TOPIC_SIZE (4 KiB) guards:
- sendMessage / sendFile: if ackLen > 1 MiB, skip ack and return
- readEventFrame: if topicLen > 4 KiB or payloadLen > 1 MiB,
  return null (treated identically to an incomplete read — benign)

All 173 existing tests pass; 1 new cap export test added.

Closes PILOT-103
@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

📊 PR Status Report

  • State: OPEN · mergeable (no conflicts)
  • CI: ✅ test (pass) · ✅ security/snyk (pass)
  • Canary: not yet triggered
  • Jira: PILOT-103 — QA/IN-REVIEW, assignee Teodor Calin, last updated 2026-05-28 09:45 +0300
  • Operator activity: none since PR opened (2026-05-28 06:45 UTC)

Awaiting operator review. Canary can be triggered on request with @matthew-pilot retry canary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant