Skip to content

connect: the other three cluster entry points parse with no size ceiling #1107

Open

Description

@sanchitmonga22

Follow-up from #1090 by @Adityakk9031 — thanks again for that PR!

What

#1090 puts a 256 MiB ceiling on ClusterActivation before it is parsed, and rac_connect_cluster_validate_activation_proto is now guarded (core/src/connect/rac_connect.cpp:957). The other three cluster entry points that came in with #723 are not:

  • rac_connect_cluster_start_proto parses at core/src/connect/rac_connect.cpp:746
  • rac_connect_cluster_join_proto parses at :838
  • rac_connect_cluster_stop_proto parses at :904

None is preceded by a request_size check. The only pre-parse validation any of them gets is rac_proto_bytes_validate (core/src/foundation/rac_proto_buffer.cpp:30-38), which rejects a null pointer with a non-zero size and anything above INT_MAX and nothing else. A crafted ClusterStartRequest carrying millions of ClusterLayerAssignment entries is therefore allocated in full before any cluster validation runs — the same untrusted-allocation shape #1078 named for tensor_data, at lower yield because no single bytes field is involved.

Why it matters

This is the same boundary #1090 just fixed, on the three sibling entry points in the same file, and it wants closing before the transport lands rather than after: #1071 (feat(connect): consume ClusterActivation.dtype in transport and SDK bindings (#1053)) is writing the receive path over these exact entry points, and a size check added once a wire shape is in use is one a peer can skip. Issue #1078 itself suggested the shape: "enforce it in parse_message — or a cluster-specific parse helper — before ParseFromArray".

Suggested approach

Put the ceiling where every cluster path must pass through it rather than on one function: a parse_cluster_message helper that applies the size check before ParseFromArray and routes all four entry points through it, so the limit is one constant applied at one boundary. #1090's test 11c (core/tests/test_connect_proto_abi.cpp:443-459) shows the pattern that pins the ordering without allocating the payload — a 1-byte buffer with an over-ceiling claimed size — and the same trick extends to ClusterStartRequest/JoinRequest/StopRequest.

Done when

  • All four cluster entry points reject an over-ceiling request before parsing it, through one shared check.
  • Each of the three new cases is covered by a test that fails if the check moves after the parse.

Not blocking #1090. @Adityakk9031, you know this code well now — you're welcome to take this one if you're interested.

Opened with help from Claude Code and Codex.

Reviewed with help from Claude Code and Codex.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    coreC++ commonsenhancementNew behaviour, not a fixp2Medium priority

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions