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

1 participant