From a9727452f3144433e7e20b6640f852e214c0c38c Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Thu, 24 Sep 2026 17:24:58 -0700 Subject: [PATCH 1/7] Serializer: store classes as references, add aliases and schema versions - Store a class value as {'__type__': 'module.Class'} and resolve it through the allowlist. Before, a class value became None without an error. - Serialize set and frozenset in sorted order. Accept a class, enum or tuple as a dict key. - Add register_class_alias(old_path, new_path) so that payloads load after a class moves or is renamed. - Add _schema_version, _migrate() and SchemaVersionError. Version 0 is not written, so existing payloads do not change. - Raise MissingClassError when a payload names a class or module that the installed code does not have. - PropagationMixin: do not call _set_parent on a class value. Co-Authored-By: Claude Opus 5.5 (1M context) --- steer_core/Mixins/Propagation.py | 19 +++- steer_core/Mixins/Serializer.py | 180 ++++++++++++++++++++++++++--- test/test_serializer_mixin.py | 187 +++++++++++++++++++++++++++++++ 3 files changed, 365 insertions(+), 21 deletions(-) diff --git a/steer_core/Mixins/Propagation.py b/steer_core/Mixins/Propagation.py index 71e2fa4..8e87941 100644 --- a/steer_core/Mixins/Propagation.py +++ b/steer_core/Mixins/Propagation.py @@ -82,6 +82,15 @@ def wrapper(self, new_value): return decorator +def _has_parent(value: Any) -> bool: + """Return True if ``value`` is an instance that can take a parent reference. + + A class stored as a value (a type reference) also has ``_set_parent``, as an + unbound function. It must not be called. + """ + return hasattr(value, '_set_parent') and not isinstance(value, type) + + class PropagationMixin: """ Mixin providing update propagation through a hierarchical object tree. @@ -320,21 +329,21 @@ def _set_parent_on_value(self, value: Any, attr_name: Optional[str] = None) -> N attr_name: The attribute name on this object that holds the value. Used for setter-based propagation. """ - if hasattr(value, '_set_parent'): + if _has_parent(value): value._set_parent(self, attr_name) - elif isinstance(value, (list, tuple)): + elif isinstance(value, (list, tuple, set, frozenset)): # For collections, items don't have a direct setter path for item in value: - if hasattr(item, '_set_parent'): + if _has_parent(item): item._set_parent(self, attr_name) elif isinstance(value, dict): # Check both keys AND values - materials can be dict keys # For dict items, they don't have a direct setter path for item in value.keys(): - if hasattr(item, '_set_parent'): + if _has_parent(item): item._set_parent(self, attr_name) for item in value.values(): - if hasattr(item, '_set_parent'): + if _has_parent(item): item._set_parent(self, attr_name) def __deepcopy__(self, memo): diff --git a/steer_core/Mixins/Serializer.py b/steer_core/Mixins/Serializer.py index 0a2f4c1..857715e 100644 --- a/steer_core/Mixins/Serializer.py +++ b/steer_core/Mixins/Serializer.py @@ -32,6 +32,25 @@ class UnsafeClassPathError(ValueError): """ +class MissingClassError(ValueError): + """A serialized payload named a class that the installed code does not have. + + The payload stores a class as a reference (its module path), never as code. + When the class was moved or renamed after the payload was written, register + the old path with :func:`register_class_alias`. Raised as a ``ValueError`` + subclass for the same reason as :class:`UnsafeClassPathError`. + """ + + +class SchemaVersionError(ValueError): + """A serialized payload is newer than the installed class can read. + + The payload's ``__schema_version__`` is above the class's + ``_schema_version``, so it may hold attributes that this code does not know. + Update the package instead of loading a partial object. + """ + + # Top-level packages whose classes may be reconstructed from a serialized # payload. Deserialization is a code-execution boundary: ``_get_class`` imports # the named module and the ``__enum__`` branch *calls* the resolved object with a @@ -56,12 +75,56 @@ def allow_class_roots(*roots: str) -> None: _ALLOWED_CLASS_ROOTS.update(roots) +# Old class path -> new class path, for classes that moved or were renamed after +# payloads were written. Filled only by code (register_class_alias), never by a +# payload, so the allowlist check applies to the new path only. +_CLASS_ALIASES: dict[str, str] = {} + + +def register_class_alias(old_path: str, new_path: str) -> None: + """Load payloads that name ``old_path`` as the class at ``new_path``. + + Call it from the package that moved the class, at import time. Aliases + chain: ``a -> b`` and ``b -> c`` load ``a`` as ``c``. + + Raises: + ValueError: If a path is malformed, or the alias would make a cycle. + """ + for path in (old_path, new_path): + if not isinstance(path, str) or '.' not in path: + raise ValueError(f"Class paths must be dotted module paths, got {path!r}") + path = new_path + while path is not None: + if path == old_path: + raise ValueError(f"Alias {old_path!r} -> {new_path!r} makes a cycle") + path = _CLASS_ALIASES.get(path) + _CLASS_ALIASES[old_path] = new_path + # A cached path can sit anywhere on a chain that this alias extends + _module_cache.clear() + + +def _resolve_alias(class_path: str) -> str: + """Follow the alias chain from ``class_path`` to its current path. + + ``register_class_alias`` refuses cycles, so the chain always ends. + """ + while class_path in _CLASS_ALIASES: + class_path = _CLASS_ALIASES[class_path] + return class_path + + +def _class_path(cls: type) -> str: + """Return the reference that a payload stores for ``cls``.""" + return f"{cls.__module__}.{cls.__name__}" + + def _get_class(class_path: str) -> type: """Resolve an allowlisted class from a serialized module path, with caching. Raises: UnsafeClassPathError: If ``class_path`` is malformed, rooted outside :data:`_ALLOWED_CLASS_ROOTS`, or does not name a class. + MissingClassError: If the module or the class no longer exists. """ cached = _module_cache.get(class_path) if cached is not None: @@ -72,6 +135,9 @@ def _get_class(class_path: str) -> type: f"Serialized data named a malformed class path: {class_path!r}" ) + saved_path = class_path + class_path = _resolve_alias(class_path) + root = class_path.split('.', 1)[0] if root not in _ALLOWED_CLASS_ROOTS: raise UnsafeClassPathError( @@ -80,8 +146,18 @@ def _get_class(class_path: str) -> type: ) module_name, class_name = class_path.rsplit('.', 1) - module = importlib.import_module(module_name) - obj = getattr(module, class_name, None) + try: + module = importlib.import_module(module_name) + except ModuleNotFoundError as error: + # Only a missing module on the path itself means "class removed". A + # missing dependency inside an existing module is a different fault. + missing = error.name or '' + if not (module_name == missing or module_name.startswith(missing + '.')): + raise + raise MissingClassError(_missing_class_message(saved_path, class_path)) from error + if not hasattr(module, class_name): + raise MissingClassError(_missing_class_message(saved_path, class_path)) + obj = getattr(module, class_name) if not isinstance(obj, type): # Guards against a path that resolves to a plain function or any other # callable — only classes are ever serialized. @@ -89,10 +165,19 @@ def _get_class(class_path: str) -> type: f"Serialized data named {class_path!r}, which is not a class." ) - _module_cache[class_path] = obj + _module_cache[saved_path] = obj return obj +def _missing_class_message(saved_path: str, class_path: str) -> str: + via = f" (alias of {saved_path!r})" if saved_path != class_path else "" + return ( + f"Serialized data named the class {class_path!r}{via}, which the installed " + "code does not have. If the class moved or was renamed, call " + "register_class_alias(old_path, new_path)." + ) + + def _get_serializable_class(class_path: str) -> type: """Resolve a class path that must name a :class:`SerializerMixin` subclass.""" obj = _get_class(class_path) @@ -121,7 +206,13 @@ def _get_enum_class(class_path: str) -> type: class SerializerMixin: - + + # Version of the attribute layout that _to_dict writes. Increase it when an + # attribute is added, removed, renamed, or changes unit, and teach _migrate + # to update older payloads. Version 0 is not written, so payloads of + # unversioned classes stay unchanged. + _schema_version: int = 0 + # Compression markers for backward compatibility _MARKER_LZ4 = b'\x02' _MARKER_ZLIB = b'\x01' @@ -139,7 +230,8 @@ def serialize(self, compress: bool = True) -> bytes: """ # Include class information for proper deserialization obj_dict = { - '_class': f"{self.__class__.__module__}.{self.__class__.__name__}", + '_class': _class_path(self.__class__), + **self._version_marker(), **self._to_dict() } data = msgpack.packb(obj_dict, use_bin_type=True) @@ -168,6 +260,11 @@ def _serialize_value(self, value: Any) -> Any: if value_type in (int, float, str, bool, bytes, type(None)): return value + # A class is stored as a reference to its module path, never as code. + # Checked before the callable test, because every class is callable. + if isinstance(value, type): + return {'__type__': _class_path(value)} + # Skip functions/methods without _to_dict if callable(value) and not hasattr(value, '_to_dict'): return None @@ -176,7 +273,8 @@ def _serialize_value(self, value: Any) -> Any: if hasattr(value, '_to_dict'): return { '__object__': True, - '_class': f"{value.__class__.__module__}.{value.__class__.__name__}", + '_class': _class_path(value.__class__), + **value._version_marker(), **value._to_dict() } @@ -196,7 +294,7 @@ def _serialize_value(self, value: Any) -> Any: if isinstance(value, Enum): return { '__enum__': True, - 'class': f"{value.__class__.__module__}.{value.__class__.__name__}", + 'class': _class_path(value.__class__), 'value': value.value } @@ -207,6 +305,12 @@ def _serialize_value(self, value: Any) -> Any: 'items': [self._serialize_value(item) for item in value] } + # Set handling. Items are sorted by their serialized form, so equal + # sets give equal payloads whatever their iteration order. + if value_type is set or value_type is frozenset: + marker = '__set__' if value_type is set else '__frozenset__' + return {marker: sorted((self._serialize_value(item) for item in value), key=repr)} + # List handling if value_type is list: return [self._serialize_value(item) for item in value] @@ -267,8 +371,9 @@ def _serialize_dict(self, d: dict) -> dict: k_serialized = self._serialize_value(k) v_serialized = self._serialize_value(v) - # Check if key or value became an object marker - k_is_obj = isinstance(k_serialized, dict) and k_serialized.get('__object__') + # A key that became any marker dict (object, type, enum, tuple) is + # not a valid msgpack map key, so it needs the item-list form + k_is_obj = isinstance(k_serialized, dict) v_is_obj = isinstance(v_serialized, dict) and v_serialized.get('__object__') if k_is_obj or v_is_obj: @@ -284,6 +389,44 @@ def _serialize_dict(self, d: dict) -> dict: else: return {k: v_ser for k, _, v_ser in items} + def _version_marker(self) -> dict: + """Return the schema-version entry for a payload, empty for version 0.""" + version = type(self)._schema_version + return {'__schema_version__': version} if version else {} + + @classmethod + def _migrate(cls, data: dict, version: int) -> dict: + """Update the payload of an older schema version to the current one. + + Called before ``_from_dict`` when the payload's version is below + ``_schema_version``. ``data`` is the raw payload: nested objects are + still marker dicts. Override it when you increase ``_schema_version``; + branch on ``version`` so that every older layout is covered. + + Args: + data: Attribute payload, without the class and version entries. + version: Schema version that wrote the payload (0 if none). + + Returns: + The payload in the current layout. + """ + return data + + @classmethod + def _load_object(cls, obj_class: type, value: dict, skip: tuple) -> Any: + """Check the schema version of one payload, migrate it, and rebuild it.""" + version = value.get('__schema_version__', 0) + current = obj_class._schema_version + if version > current: + raise SchemaVersionError( + f"Serialized {_class_path(obj_class)} has schema version {version}, " + f"but the installed class reads up to version {current}." + ) + obj_data = {k: v for k, v in value.items() if k not in skip} + if version < current: + obj_data = obj_class._migrate(obj_data, version) + return obj_class._from_dict(obj_data) + def _to_dict(self) -> dict: """ Convert object to dictionary for serialization. @@ -298,8 +441,8 @@ def _to_dict(self) -> dict: # Skip _parent - PropagationMixin rebuilds it via _from_dict if key == '_parent': continue - # Skip non-serializable callables - if callable(value) and not hasattr(value, '_to_dict'): + # Skip non-serializable callables (a class is kept as a reference) + if callable(value) and not hasattr(value, '_to_dict') and not isinstance(value, type): continue result[key] = self._serialize_value(value) return result @@ -332,9 +475,7 @@ def deserialize(cls: type[T], data: bytes) -> T: # Use stored class information if available if '_class' in obj_dict: actual_cls = _get_serializable_class(obj_dict['_class']) - # Remove class marker before reconstructing - obj_data = {k: v for k, v in obj_dict.items() if k != '_class'} - return actual_cls._from_dict(obj_data) + return cls._load_object(actual_cls, obj_dict, ('_class', '__schema_version__')) else: # Fallback for backward compatibility return cls._from_dict(obj_dict) @@ -359,14 +500,21 @@ def _deserialize_value(cls, value: Any) -> Any: if '__object__' in value: # Reconstruct regular object using cached class lookup obj_class = _get_serializable_class(value['_class']) - obj_data = {k: v for k, v in value.items() if k not in ('__object__', '_class')} - return obj_class._from_dict(obj_data) + return cls._load_object( + obj_class, value, ('__object__', '_class', '__schema_version__') + ) + elif '__type__' in value: + return _get_class(value['__type__']) elif '__datetime__' in value: return datetime.fromisoformat(value['__datetime__']) elif '__enum__' in value: # Reconstruct enum using cached class lookup enum_class = _get_enum_class(value['class']) return enum_class(value['value']) + elif '__set__' in value: + return {cls._deserialize_value(item) for item in value['__set__']} + elif '__frozenset__' in value: + return frozenset(cls._deserialize_value(item) for item in value['__frozenset__']) elif '__tuple__' in value: # Recursively reconstruct tuple items return tuple(cls._deserialize_value(item) for item in value['items']) diff --git a/test/test_serializer_mixin.py b/test/test_serializer_mixin.py index b8152c1..6a75d91 100644 --- a/test/test_serializer_mixin.py +++ b/test/test_serializer_mixin.py @@ -11,10 +11,15 @@ import pandas as pd import pytest +from steer_core.Mixins import Serializer +from steer_core.Mixins.Propagation import PropagationMixin from steer_core.Mixins.Serializer import ( + MissingClassError, + SchemaVersionError, SerializerMixin, UnsafeClassPathError, allow_class_roots, + register_class_alias, ) # Deserialization only reconstructs classes rooted in an allowlisted package (see @@ -253,3 +258,185 @@ def test_list_serialization(self): s = SimpleSerializable() result = s._serialize_value([1, "two", 3.0]) assert result == [1, "two", 3.0] + + +class Holder(SerializerMixin): + """Holds one arbitrary value, to test how a value type round-trips.""" + + def __init__(self, value=None): + self.value = value + + +class ParentHolder(PropagationMixin, SerializerMixin): + """A propagating object, to test that a class value is not given a parent.""" + + def __init__(self, value=None): + self._parent = None + self._value = value + + +def _round_trip(value): + return Holder.deserialize(Holder(value).serialize()).value + + +@pytest.fixture +def clean_aliases(): + """Restore the global alias table and class cache after a test.""" + aliases = dict(Serializer._CLASS_ALIASES) + cache = dict(Serializer._module_cache) + yield + Serializer._CLASS_ALIASES.clear() + Serializer._CLASS_ALIASES.update(aliases) + Serializer._module_cache.clear() + Serializer._module_cache.update(cache) + + +class TestTypeReferences: + + def test_type_value_round_trips_as_the_same_class(self): + assert _round_trip(SimpleSerializable) is SimpleSerializable + + def test_type_is_stored_as_a_path_not_as_code(self): + payload = Holder()._serialize_value(Color) + assert payload == {'__type__': f"{Color.__module__}.Color"} + + def test_serializable_class_as_value_is_not_called(self): + # A SerializerMixin subclass has _to_dict, but as a class it must stay a reference + assert _round_trip([SimpleSerializable, Holder]) == [SimpleSerializable, Holder] + + def test_classes_as_dict_keys(self): + labour = {SimpleSerializable: 1.5, Holder: 0.25} + assert _round_trip(labour) == labour + + def test_tuple_and_enum_keys(self): + value = {(1, 2): "a", Color.RED: "b"} + assert _round_trip(value) == value + + def test_type_outside_allowlist_is_rejected_on_load(self): + data = Holder(datetime).serialize() + with pytest.raises(UnsafeClassPathError): + Holder.deserialize(data) + + def test_class_value_does_not_get_a_parent(self): + restored = ParentHolder.deserialize(ParentHolder(ParentHolder).serialize()) + assert restored._value is ParentHolder + + +class TestSets: + + def test_set_round_trip(self): + value = {1, "two", Color.BLUE} + restored = _round_trip(value) + assert restored == value and type(restored) is set + + def test_frozenset_of_types_round_trip(self): + value = frozenset({SimpleSerializable, Holder}) + restored = _round_trip(value) + assert restored == value and type(restored) is frozenset + + def test_equal_sets_give_equal_payloads(self): + first = Holder({"b", "a", "c"})._serialize_value({"b", "a", "c"}) + second = Holder()._serialize_value({"c", "a", "b"}) + assert first == second + + +class TestClassAliases: + + def test_old_path_loads_as_the_new_class(self, clean_aliases): + old = f"{__name__}.OldName" + register_class_alias(old, f"{__name__}.SimpleSerializable") + assert Serializer._get_class(old) is SimpleSerializable + + def test_aliases_chain(self, clean_aliases): + register_class_alias(f"{__name__}.A", f"{__name__}.B") + register_class_alias(f"{__name__}.B", f"{__name__}.SimpleSerializable") + assert Serializer._get_class(f"{__name__}.A") is SimpleSerializable + + def test_cycle_is_rejected(self, clean_aliases): + register_class_alias(f"{__name__}.A", f"{__name__}.B") + with pytest.raises(ValueError, match="cycle"): + register_class_alias(f"{__name__}.B", f"{__name__}.A") + + def test_alias_target_is_still_allowlisted(self, clean_aliases): + register_class_alias(f"{__name__}.Old", "os.system") + with pytest.raises(UnsafeClassPathError): + Serializer._get_class(f"{__name__}.Old") + + @pytest.mark.parametrize("path", ["", "nodot", None]) + def test_malformed_alias_is_rejected(self, path, clean_aliases): + with pytest.raises(ValueError): + register_class_alias(path, f"{__name__}.SimpleSerializable") + + def test_alias_replaces_a_cached_path(self, clean_aliases): + path = f"{__name__}.SimpleSerializable" + Serializer._get_class(path) + register_class_alias(path, f"{__name__}.Holder") + assert Serializer._get_class(path) is Holder + + +class TestMissingClass: + + def test_removed_class_names_the_path(self): + with pytest.raises(MissingClassError, match="register_class_alias"): + Serializer._get_class(f"{__name__}.RemovedClass") + + def test_removed_module(self): + with pytest.raises(MissingClassError): + Serializer._get_class(f"{__name__}.removed_module.RemovedClass") + + def test_missing_class_is_a_value_error(self): + assert issubclass(MissingClassError, ValueError) + + +class Versioned(SerializerMixin): + """Version 2: ``length`` in m. Version 1 had ``length_mm``; version 0 had ``size_mm``.""" + + _schema_version = 2 + + def __init__(self, length=1.0): + self.length = length + + @classmethod + def _migrate(cls, data, version): + if version < 1: + data['length_mm'] = data.pop('size_mm') + if version < 2: + data['length'] = data.pop('length_mm') / 1000 + return data + + +def _payload_of(obj_dict): + return SerializerMixin._MARKER_NONE + msgpack.packb(obj_dict, use_bin_type=True) + + +class TestSchemaVersion: + + def test_version_zero_is_not_written(self): + raw = msgpack.unpackb(SimpleSerializable().serialize(compress=False)[1:], raw=False) + assert '__schema_version__' not in raw + + def test_current_version_is_written_and_loads(self): + raw = msgpack.unpackb(Versioned(2.0).serialize(compress=False)[1:], raw=False) + assert raw['__schema_version__'] == 2 + assert Versioned.deserialize(Versioned(2.0).serialize()).length == 2.0 + + @pytest.mark.parametrize("version, attributes", [ + (0, {'size_mm': 1500.0}), + (1, {'length_mm': 1500.0}), + ]) + def test_older_payload_is_migrated(self, version, attributes): + payload = {'_class': f"{__name__}.Versioned", **attributes} + if version: + payload['__schema_version__'] = version + restored = Versioned.deserialize(_payload_of(payload)) + assert restored.__dict__ == {'length': 1.5} + + def test_nested_object_is_migrated(self): + nested = {'__object__': True, '_class': f"{__name__}.Versioned", 'size_mm': 500.0} + payload = {'_class': f"{__name__}.Holder", 'value': nested} + assert Holder.deserialize(_payload_of(payload)).value.length == 0.5 + + def test_newer_payload_is_rejected(self): + payload = {'_class': f"{__name__}.Versioned", '__schema_version__': 3, 'length': 1.0} + with pytest.raises(SchemaVersionError, match="version 3"): + Versioned.deserialize(_payload_of(payload)) From f76020783d414f496e49062151b2d4074bd8f49d Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 11:01:16 -0700 Subject: [PATCH 2/7] Fix RCE: refuse object-dtype arrays in msgpack deserialization msgpack_numpy.decode calls pickle.loads on object-dtype ('O') numpy arrays. m.patch() makes it the global msgpack object_hook, so any msgpack.unpackb in the process runs pickle.loads on such an array. pickle.loads runs arbitrary code, so an untrusted payload (for example an uploaded .ocd design) could execute code during unpack, 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. Capture the unpatched msgpack.unpackb before m.patch(), then re-override msgpack.unpackb/loads with a safe reader that refuses object-dtype arrays and delegates all other maps (numeric/void arrays, complex numbers, plain dicts) to the normal decoder. Writing (packb) is unchanged. No cell design uses object-dtype arrays (verified against the OpenCell database: 45895 arrays, all numeric, zero object arrays), so the refusal breaks no real design. Co-Authored-By: Claude Opus 5.5 (1M context) --- pyproject.toml | 1 + steer_core/Mixins/Serializer.py | 54 ++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) 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 0a2f4c1..db1bb0e 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 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. From 792be67e579bc7900e1fa6803d1647d0e28b8a52 Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 14:40:16 -0700 Subject: [PATCH 3/7] Bump version to 0.2.24 and add changelog for the RCE fix Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 16 ++++++++++++++++ steer_core/__init__.py | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d14975b..372cd66 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.24] - 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.23] - 2026-09-14 ### Fixed diff --git a/steer_core/__init__.py b/steer_core/__init__.py index ff415e9..6a05b26 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.23" +__version__ = "0.2.24" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin From 637ca544bcb2d354ba8e696bcdaef80a9ccea170 Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 15:31:54 -0700 Subject: [PATCH 4/7] Bump version to 0.2.25 0.2.24 is already on PyPI and is now used by PR #48. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- steer_core/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 372cd66..55d2e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ 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.24] - 2026-09-29 +## [0.2.25] - 2026-09-29 ### Security - **Remote code execution through msgpack deserialization.** `msgpack_numpy`'s diff --git a/steer_core/__init__.py b/steer_core/__init__.py index 6a05b26..e947530 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.24" +__version__ = "0.2.25" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin From 48f4a0e37e78ad9e91f05c9e607b91984bdcac65 Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 15:31:54 -0700 Subject: [PATCH 5/7] Bump version to 0.2.24 Co-Authored-By: Claude Opus 5.5 --- steer_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/steer_core/__init__.py b/steer_core/__init__.py index ff415e9..6a05b26 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.23" +__version__ = "0.2.24" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin From fea9b4deecab8fe32e433baaafbfca39084af080 Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 15:33:40 -0700 Subject: [PATCH 6/7] Bump version to 0.2.26 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- steer_core/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55d2e6c..6c5f69d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ 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.25] - 2026-09-29 +## [0.2.26] - 2026-09-29 ### Security - **Remote code execution through msgpack deserialization.** `msgpack_numpy`'s 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 From c3ad05cf5adbdd4c2fe05ded608e4c9e779ccd60 Mon Sep 17 00:00:00 2001 From: 0x1r1s Date: Tue, 29 Sep 2026 15:33:40 -0700 Subject: [PATCH 7/7] Bump version to 0.2.25 Co-Authored-By: Claude Opus 5.5 --- steer_core/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/steer_core/__init__.py b/steer_core/__init__.py index 6a05b26..e947530 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.24" +__version__ = "0.2.25" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin