Skip to content

Remote code execution: LoRAPatch.load_patch() unpickles a peer-supplied file path #3

Description

@G2P2

protocol/lora_patch.py is documented as the mechanism for "sharing and managing LoRA patches across the symphony network". That means the object is reconstructed from data that arrived over the wire — and then handed straight to torch.load:

        if not os.path.exists(self.patch_path):
            raise FileNotFoundError(f"Patch file not found: {self.patch_path}")

        try:
            return torch.load(self.patch_path)

protocol/lora_patch.py:127-131

torch.load without weights_only=True is pickle.load with extra steps. A pickle stream can carry a __reduce__ that runs arbitrary code at deserialization time. There is no allowlist, no signature check, no hash pinning.

The path itself is also untrusted. from_dict takes it verbatim from the incoming dictionary:

            patch_path=data.get("patch_path", ""),

protocol/lora_patch.py:80

Failure scenario

  • Trigger: a peer node sends a patch-share message whose payload is {"source_id": "friendly", "patch_path": "/home/dev/.cache/evil.pt", "layer_names": []}. NetworkAdapter._deserialize_data returns the dict as-is (infra/network_adapter.py:217-223), LoRAPatch.from_dict builds the object, and something downstream calls load_patch().
  • Observed: the pickle's reduce hook executes in the receiving process. That process has OPENAI_API_KEY / OPENROUTER_API_KEY in its environment (core/embeddings.py:201-202 reads them), so the cheapest possible payload is "exfiltrate both keys and exit quietly". Nothing in the logs looks unusual — load_patch returns a plausible state dict.
  • Expected: an untrusted patch should never be able to run code, and should never be able to name an arbitrary absolute path on the receiver's disk.

Suggested fix

Three independent hardenings, all cheap:

  1. torch.load(self.patch_path, map_location="cpu", weights_only=True). This is the single highest-value line change in the repo. It rejects any non-tensor pickle opcode outright.
  2. Treat patch_path as a name, not a path. from_dict should reject anything containing os.sep or .. and resolve it under the node's own shared_patches/ root — note save_patch already constructs safe filenames itself (protocol/lora_patch.py:106), so the receive side is the only asymmetry.
  3. layer_names should be validated against the local model's LoRA layer set before the patch is merged, otherwise a malicious peer can poison layers it was never authorised to touch.

Given that core/identity.py already implements Ed25519 signing, signing the patch bytes and verifying against the sender's registered key would close this properly — the primitives exist, they just are not wired into this path.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions