Skip to content

fix(model_conversion): use weights_only=True in OptimizationConverter (CWE-502)#98

Open
sebastiondev wants to merge 1 commit into
multimindlab:developfrom
sebastiondev:fix/cwe502-optimization-torch-ee0c
Open

fix(model_conversion): use weights_only=True in OptimizationConverter (CWE-502)#98
sebastiondev wants to merge 1 commit into
multimindlab:developfrom
sebastiondev:fix/cwe502-optimization-torch-ee0c

Conversation

@sebastiondev

Copy link
Copy Markdown

What does this PR do?

Fixes an insecure deserialization (CWE-502) vulnerability in multimind/model_conversion/optimization.py. The OptimizationConverter class loaded PyTorch checkpoints via torch.load(model_path) in three places (convert, validate, get_metadata) without passing weights_only=True. On PyTorch < 2.6 — which is permitted by the SDK's torch>=2.0.0 pin — torch.load defaults to full pickle deserialization, so a crafted .pt file executes arbitrary Python (RCE) the moment it is opened.

All three call sites now route through a small helper, _safe_torch_load, which passes weights_only=True explicitly. This restricts unpickling to an allow-list of tensor / storage / primitive types and rejects __reduce__-based payloads, independent of the user's installed torch version.

Vulnerability summary

  • CWE: 502 — Deserialization of Untrusted Data
  • File / functions: multimind/model_conversion/optimization.pyOptimizationConverter.convert, .validate, .get_metadata
  • Impact: Remote code execution on the machine running the SDK when a malicious checkpoint is loaded
  • Trigger: Any code path that hands an attacker-influenced model_path to OptimizationConverter — e.g. checkpoints pulled from a shared model registry, user-uploaded files in a pipeline, or a downloaded pretrained model

Fix rationale

  • weights_only=True is the upstream-recommended mitigation (see PyTorch security advisory GHSA-pg7h-5qx3-wjr3 and the torch.load docs).
  • We do not bump the torch>=2.0.0 pin to >=2.6, because that would be a breaking change for users on older Python / CUDA combinations. Passing the flag explicitly gives safe behaviour on every supported torch version.
  • The helper is defined once so future callers in the same module inherit the safe default and the rationale is documented in one place.
  • No public API change; the three method signatures are unchanged.

Proof of concept

Reproduces on any environment with torch<2.6 installed (e.g. pip install "torch==2.5.1"):

# build_malicious_ckpt.py
import pickle, os

class RCE:
    def __reduce__(self):
        return (os.system, ("touch /tmp/multimind_pwned",))

with open("evil.pt", "wb") as f:
    pickle.dump(RCE(), f)
# trigger.py — before the fix
from multimind.model_conversion.optimization import OptimizationConverter
OptimizationConverter().validate("evil.pt")   # /tmp/multimind_pwned appears

After the fix, the same call raises pickle.UnpicklingError: Weights only load failed ... GLOBAL posix.system is forbidden, no file is created, and validate returns False via its existing except Exception guard. convert and get_metadata similarly refuse the payload instead of executing it.

Testing

  • Reproduced RCE on torch==2.5.1 against the pre-fix code (marker file created).
  • Verified the post-fix code rejects the same payload with UnpicklingError on torch==2.5.1 and torch==2.2.2.
  • Loading a legitimate state_dict checkpoint (torch.save(model.state_dict(), ...)) still succeeds through _safe_torch_load.
  • git diff --stat shows a single file changed, +21/-3 — no unrelated edits.

Adversarial review

Before submitting we tried to disprove the finding. The pinned range torch>=2.0.0 in requirements.txt really does allow vulnerable versions (2.0–2.5), and torch.load's weights_only default is False on all of them; upgrading torch alone is not a mitigation the SDK can rely on. There is no sanitisation between model_path and torch.load, and validate's except Exception swallows the unpickling error but only after pickle has already executed the payload, so it does not prevent RCE. We also checked that OptimizationConverter is part of the public conversion API (exposed via multimind.model_conversion) and reachable from user-supplied paths, not just internal fixtures.

Checklist

  • Minimal, targeted change (single file, single concern)
  • No breaking changes — public signatures unchanged
  • Reproducer verified before and after the fix
  • CHANGELOG.md — happy to add an entry under a "Security" heading if you'd like; left out to keep the diff minimal

Discovered by the Sebastion AI GitHub App.

… (CWE-502)

OptimizationConverter.convert/validate/get_metadata call torch.load()
without passing weights_only. On PyTorch < 2.6 the default was
weights_only=False, which pickle-deserializes the input file and
executes any __reduce__ payload it contains. The repo pins only
torch>=2.0.0, so this reaches RCE on any 2.0.x–2.5.x runtime whenever
the checkpoint path is not fully trusted (e.g., a downloaded snapshot,
a shared model registry, an uploaded checkpoint, or a pipeline stage
consuming a converter's output).

Route all three call sites through a small _safe_torch_load() helper
that passes weights_only=True explicitly. This restricts unpickling to
tensor/storage/primitive types on every supported torch version and
refuses arbitrary class instances at load time.

No API changes.
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