Skip to content

fix(security): stop pickling the multinode ip handshake (GH-1413) - #1458

Open
DivyaNarahari97 wants to merge 1 commit into
ModelTC:mainfrom
DivyaNarahari97:fix/multinode-ip-handshake-no-pickle
Open

fix(security): stop pickling the multinode ip handshake (GH-1413)#1458
DivyaNarahari97 wants to merge 1 commit into
ModelTC:mainfrom
DivyaNarahari97:fix/multinode-ip-handshake-no-pickle

Conversation

@DivyaNarahari97

Copy link
Copy Markdown

The head node's startup handshake bound a ZMQ PULL socket on tcp://* and read from it with recv_pyobj(), which is pickle.loads() on unauthenticated network data. Any host able to reach that port could send a crafted pickle and execute arbitrary code on the head node -- the CVE-2025-32444 pattern.

The wildcard bind itself is not the defect to fix: child nodes connect to this port from other machines, so it has to accept remote connections. Every intra-node socket in the codebase already binds 127.0.0.1 explicitly; these multinode sockets are deliberately reachable. The defect is using pickle as the wire format for data that arrives from the network.

The payload here is a single IP string, so pickle buys nothing. Send it as utf-8 bytes and validate on receipt:

  • reject payloads over 64 bytes (an IPv6 address maxes out at 45),
  • decode as utf-8,
  • require ipaddress.ip_address() to accept it.

This removes the deserialization path entirely; there is no longer any object graph to reconstruct. A malformed payload now fails startup with a ValueError instead of being written into args.child_ips and surfacing later as a confusing connection error. Also closes the socket via try/finally so a rejected payload cannot leak it.

Tests cover valid v4/v6 addresses and malformed input, and assert that pickle payloads across protocols 0/1/2/HIGHEST are rejected without executing. One case is deliberately a compact protocol-0 pickle: it is pure ASCII and under the size cap, so it clears both cheaper checks and proves ip_address() is what actually stops it.

Note this changes the wire format, so all nodes in a cluster must run matching versions -- normal for multinode deployments off one image.

Remaining exposure, not addressed here: HttpServerManager.loop_for_request does recv_pyobj() on a wildcard-bound socket during serving. Its payload is a full (prompt, SamplingParams, MultimodalParams) tuple, so removing pickle there needs a real serialization design and maintainer input.

The head node's startup handshake bound a ZMQ PULL socket on tcp://* and
read from it with recv_pyobj(), which is pickle.loads() on unauthenticated
network data. Any host able to reach that port could send a crafted pickle
and execute arbitrary code on the head node -- the CVE-2025-32444 pattern.

The wildcard bind itself is not the defect to fix: child nodes connect to
this port from other machines, so it has to accept remote connections.
Every intra-node socket in the codebase already binds 127.0.0.1
explicitly; these multinode sockets are deliberately reachable. The
defect is using pickle as the wire format for data that arrives from the
network.

The payload here is a single IP string, so pickle buys nothing. Send it
as utf-8 bytes and validate on receipt:

  - reject payloads over 64 bytes (an IPv6 address maxes out at 45),
  - decode as utf-8,
  - require ipaddress.ip_address() to accept it.

This removes the deserialization path entirely; there is no longer any
object graph to reconstruct. A malformed payload now fails startup with a
ValueError instead of being written into args.child_ips and surfacing
later as a confusing connection error. Also closes the socket via
try/finally so a rejected payload cannot leak it.

Tests cover valid v4/v6 addresses and malformed input, and assert that
pickle payloads across protocols 0/1/2/HIGHEST are rejected without
executing. One case is deliberately a compact protocol-0 pickle: it is
pure ASCII and under the size cap, so it clears both cheaper checks and
proves ip_address() is what actually stops it.

Note this changes the wire format, so all nodes in a cluster must run
matching versions -- normal for multinode deployments off one image.

Remaining exposure, not addressed here: HttpServerManager.loop_for_request
does recv_pyobj() on a wildcard-bound socket during serving. Its payload
is a full (prompt, SamplingParams, MultimodalParams) tuple, so removing
pickle there needs a real serialization design and maintainer input.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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