diff --git a/CHANGELOG.md b/CHANGELOG.md index caad5ea..347c33b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.2.26] - 2026-09-29 + +### Security +- **Remote code execution through msgpack deserialization.** `msgpack_numpy`'s + decoder calls `pickle.loads` on object-dtype (`kind == 'O'`) numpy arrays, and + `m.patch()` installs it as the global msgpack `object_hook`. So any + `msgpack.unpackb` in the process — including deserialization of an untrusted + payload such as an uploaded `.ocd` design — ran `pickle.loads`, which executes + arbitrary code, before any class-allowlist check. The patched reader also + wraps a caller-supplied `object_hook` and runs `decode` first, so a hook + cannot make unpack safe. Fixed by capturing the unpatched `msgpack.unpackb` + before `m.patch()` and re-overriding `msgpack.unpackb`/`loads` with a safe + reader that refuses object-dtype arrays (`UnsafeObjectArrayError`) and + delegates every other map to the normal decoder. Writing (`packb`) is + unchanged. No cell design uses object-dtype arrays. + ## [0.2.25] - 2026-09-29 ### Added diff --git a/pyproject.toml b/pyproject.toml index e275d71..6d9f08e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,7 @@ requires-python = ">=3.10" license = {text = "AGPL-3.0-or-later"} authors = [ {name = "Nicholas Siemons", email = "nsiemons@stanford.edu"}, + {name = "Antoine Breton", email = "abreton@stanford.edu"}, ] maintainers = [ {name = "Nicholas Siemons", email = "nsiemons@stanford.edu"}, diff --git a/steer_core/Mixins/Serializer.py b/steer_core/Mixins/Serializer.py index e0902e4..b6a8bac 100644 --- a/steer_core/Mixins/Serializer.py +++ b/steer_core/Mixins/Serializer.py @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: 2024-2026 Stanford University # SPDX-License-Identifier: AGPL-3.0-or-later +import functools import importlib import msgpack import msgpack_numpy as m @@ -11,9 +12,60 @@ from datetime import datetime, timedelta from enum import Enum -# Patch msgpack for numpy support once at module import +# Capture the unpatched msgpack reader BEFORE m.patch() replaces it. m.patch() +# installs msgpack_numpy.decode as the global object_hook, and that decoder calls +# pickle.loads on object-dtype ('O') numpy arrays. pickle.loads runs arbitrary +# code, so an untrusted payload could execute code during unpack. The patched +# reader also wraps any object_hook a caller passes (it runs decode *first*), so +# a caller cannot make unpack safe by passing a hook — the reader itself must be +# replaced. See _safe_unpackb below. +_raw_unpackb = msgpack.unpackb + +# Patch msgpack for numpy support once at module import (encoding + decoding). m.patch() + +class UnsafeObjectArrayError(ValueError): + """A payload asked to decode an object-dtype ('O') numpy array. + + That decode path calls ``pickle.loads``, which runs arbitrary code. No cell + design uses object-dtype arrays, so this shape only appears in a hostile + payload and is refused. + """ + + +def _safe_decode(obj: Any, chain: Any = None) -> Any: + """Object hook that decodes numeric numpy arrays but refuses object arrays. + + ``msgpack_numpy.decode`` calls ``pickle.loads`` when a map declares an + object-dtype array (``b'kind' == b'O'``). This wrapper rejects that shape + before ``decode`` can reach the pickle branch, and delegates every other map + (numeric/void arrays, complex numbers, plain dicts) to the normal decoder. + """ + if isinstance(obj, dict) and b'nd' in obj and obj.get(b'kind') == b'O': + raise UnsafeObjectArrayError( + "object-dtype ('O') numpy arrays are not allowed" + ) + return m.decode(obj, chain=chain) + + +def _safe_unpackb(packed: bytes, **kwargs: Any) -> Any: + """Drop-in replacement for the patched ``msgpack.unpackb`` that cannot pickle. + + Uses the unpatched reader with :func:`_safe_decode` as the object hook, so + numeric numpy arrays still round-trip while the pickle path stays closed. + """ + chain = kwargs.pop('object_hook', None) + kwargs['object_hook'] = functools.partial(_safe_decode, chain=chain) + return _raw_unpackb(packed, **kwargs) + + +# Re-override the readers that m.patch() made unsafe. Writing (packb) is +# unchanged, so serialization of numpy arrays still works. Every msgpack read in +# the process now refuses object-dtype arrays instead of unpickling them. +msgpack.unpackb = _safe_unpackb +msgpack.loads = _safe_unpackb + # Module-level cache for imported modules (avoids repeated importlib calls). # Only ever holds paths that passed _get_class's allowlist + type checks, so a # cache hit is safe to return without re-validating. diff --git a/steer_core/__init__.py b/steer_core/__init__.py index e947530..ceed039 100644 --- a/steer_core/__init__.py +++ b/steer_core/__init__.py @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2024-2026 Stanford University # SPDX-License-Identifier: AGPL-3.0-or-later -__version__ = "0.2.25" +__version__ = "0.2.26" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin