diff --git a/docs/claude/python/NORMALIZATION.md b/docs/claude/python/NORMALIZATION.md index 61528bc8efa..06e3690998c 100644 --- a/docs/claude/python/NORMALIZATION.md +++ b/docs/claude/python/NORMALIZATION.md @@ -121,6 +121,29 @@ ArcticDB uses a type-based dispatch system. Normalizers inherit from `Normalizer | TimeFrame | `TimeFrameNormalizer` | | Arbitrary objects | `MsgPackNormalizer` (fallback) | +## Pickle Reads (Security) + +Non-normalizable objects and user-defined metadata are stored by `MsgPackNormalizer` as pickle +bytes wrapped in a msgpack `ExtType` (`PY_PICKLE_2`/`PY_PICKLE_3`). Unpickling on read executes +arbitrary code embedded in the stored bytes, so a writer to shared storage could achieve remote +code execution in a reader's process. + +Reading such payloads is therefore **disabled by default**. When `MsgPackNormalizer._ext_hook` +encounters a `PY_PICKLE_*` code it calls `_check_pickle_read_allowed`, which raises +`UnsafePickleReadError` unless pickle reads have been explicitly enabled. This gates every read +path (whole-symbol denormalize and `denormalize_user_metadata`). Note `strict_mode` only blocks +*writing* new pickles; it does not affect the read gate. + +Opt in (only when every writer to the library is trusted), highest precedence first: + +| Mechanism | Scope | +|-----------|-------| +| `MsgPackNormalizer.allow_pickle_reads = True` | Per-instance override | +| `arcticdb.set_allow_pickle_reads(True)` (`None` to defer to env) | Process-wide | +| `ARCTICDB_ALLOW_PICKLE_READS=1` env var | Process-wide default | + +See `_normalization.py:allow_pickle_reads` / `set_allow_pickle_reads`. + ## String Handling ### Dynamic Strings diff --git a/docs/mkdocs/docs/faq.md b/docs/mkdocs/docs/faq.md index aaad89251a7..1cff3fa4f77 100644 --- a/docs/mkdocs/docs/faq.md +++ b/docs/mkdocs/docs/faq.md @@ -128,6 +128,17 @@ ArcticDB is only able to store the following data types natively: Note that ArcticDB cannot efficiently store custom Python objects, even if inserted into a Pandas DataFrames/NumPy array. Pickled data cannot be index or column-sliced, and neither `update` nor `append` primitives will function on pickled data. +!!! warning "Reading pickled data executes arbitrary code" + + Reading a pickled symbol or pickled user-defined metadata deserializes it with Python's + `pickle`, which **executes arbitrary code embedded in the stored bytes**. Anyone who can write + to a library you read can therefore run code in your process (remote code execution). For this + reason ArcticDB **refuses to read pickled data by default** and raises `UnsafePickleReadError`. + + Only enable pickle reads if you trust every writer to the libraries you read, via + `arcticdb.set_allow_pickle_reads(True)` or by setting the `ARCTICDB_ALLOW_PICKLE_READS=1` + environment variable. This does not affect reading natively-stored (non-pickled) data. + ### *How does indexing work in ArcticDB?* See the [Getting Started](index.md#reading-and-writing-data) page for details of supported index types. diff --git a/python/arcticdb/__init__.py b/python/arcticdb/__init__.py index c27fa508f4c..fc490d68625 100644 --- a/python/arcticdb/__init__.py +++ b/python/arcticdb/__init__.py @@ -27,6 +27,8 @@ WriteMetadataPayload, ) from arcticdb.version_store.admin_tools import KeyType, Size +from arcticdb.version_store._normalization import set_allow_pickle_reads +from arcticdb.exceptions import UnsafePickleReadError set_config_from_env_vars(_os.environ) diff --git a/python/arcticdb/exceptions.py b/python/arcticdb/exceptions.py index 0b8c78aae46..bd858fd7f2c 100644 --- a/python/arcticdb/exceptions.py +++ b/python/arcticdb/exceptions.py @@ -24,6 +24,23 @@ class ArcticDbNotYetImplemented(ArcticException): ArcticNativeNotYetImplemented = ArcticDbNotYetImplemented +class UnsafePickleReadError(ArcticException): + """Raised on read when a stored symbol or user-defined metadata payload can only be + decoded by unpickling, but pickle reads have not been explicitly enabled. + + ArcticDB stores non-normalizable objects and user metadata as pickle bytes. Unpickling + executes arbitrary code embedded in the payload, so a writer to shared storage could plant + a malicious payload that runs in the reader's process (remote code execution). Reading such + payloads is therefore disabled by default. + + Enable it only if you trust every writer to the libraries you read, either process-wide via + ``arcticdb.set_allow_pickle_reads(True)`` / the ``ARCTICDB_ALLOW_PICKLE_READS`` environment + variable. + """ + + pass + + class LibraryNotFound(ArcticException): pass diff --git a/python/arcticdb/version_store/_normalization.py b/python/arcticdb/version_store/_normalization.py index 2a8fda3124b..ee23ce0abaa 100644 --- a/python/arcticdb/version_store/_normalization.py +++ b/python/arcticdb/version_store/_normalization.py @@ -38,6 +38,7 @@ ArcticDbNotYetImplemented, NormalizationException, UnsortedDataException, + UnsafePickleReadError, ) from arcticdb.supported_types import DateRangeInput, time_types as supported_time_types from arcticdb.util._versions import IS_PANDAS_TWO, IS_PANDAS_ZERO @@ -1367,6 +1368,42 @@ def normalize( ) +_ALLOW_PICKLE_READS_ENV_VAR = "ARCTICDB_ALLOW_PICKLE_READS" +# None => defer to the environment variable. Set explicitly via set_allow_pickle_reads(). +_allow_pickle_reads_override = None + + +def set_allow_pickle_reads(allow): + """Control whether ArcticDB is allowed to unpickle stored symbols/metadata on read. + + Reading a pickled symbol or user-defined metadata deserializes it with ``pickle.load``, + which executes arbitrary code embedded in the stored bytes. A writer to shared storage + could therefore plant a payload that runs in the reader's process (remote code execution), + so this is disabled by default and reading such a payload raises ``UnsafePickleReadError``. + + Enable this only if you trust every writer to the libraries you read. + + :param allow: ``True``/``False`` to force enable/disable, or ``None`` to defer to the + ``ARCTICDB_ALLOW_PICKLE_READS`` environment variable. + """ + global _allow_pickle_reads_override + _allow_pickle_reads_override = None if allow is None else bool(allow) + + +def _env_allows_pickle_reads(): + val = os.environ.get(_ALLOW_PICKLE_READS_ENV_VAR) + if val is None: + return False + return val.strip().lower() in ("1", "true", "yes", "on") + + +def allow_pickle_reads(): + """Return whether unpickling stored data on read is currently permitted.""" + if _allow_pickle_reads_override is not None: + return _allow_pickle_reads_override + return _env_allows_pickle_reads() + + class MsgPackNormalizer(Normalizer): """ Fall back plan for the time being to store arbitrary data @@ -1374,6 +1411,8 @@ class MsgPackNormalizer(Normalizer): def __init__(self, cfg=None): self.strict_mode = cfg.strict_mode if cfg is not None else False + # None => defer to the global allow_pickle_reads() setting. + self.allow_pickle_reads = None def normalize(self, obj, **kwargs): disallow_pickle = kwargs.get("disallow_pickle", None) @@ -1444,17 +1483,35 @@ def _ext_hook(self, code, data): return pd.Timedelta(data).to_pytimedelta() if code == MsgPackSerialization.PY_PICKLE_2: + self._check_pickle_read_allowed() # If stored in Python2 we want to use raw while unpacking. # https://github.com/msgpack/msgpack-python/blob/master/msgpack/_unpacker.pyx#L230 data = unpackb(data, raw=True) return Pickler.read(data) if code == MsgPackSerialization.PY_PICKLE_3: + self._check_pickle_read_allowed() data = unpackb(data, raw=False) return Pickler.read(data) return ExtType(code, data) + def _pickle_reads_allowed(self): + # Instance override takes priority, otherwise defer to the global setting. + return self.allow_pickle_reads if self.allow_pickle_reads is not None else allow_pickle_reads() + + def _check_pickle_read_allowed(self): + if not self._pickle_reads_allowed(): + raise UnsafePickleReadError( + "Refusing to unpickle stored data on read because pickle reads are disabled. " + "This payload was stored as a pickle (e.g. a pickled symbol or user-defined " + "metadata) and decoding it would execute arbitrary code embedded in the stored " + "bytes, which is a remote code execution risk if any untrusted writer can reach " + "the storage. If you trust every writer to this library, enable pickle reads via " + "arcticdb.set_allow_pickle_reads(True) or by setting the " + f"{_ALLOW_PICKLE_READS_ENV_VAR}=1 environment variable." + ) + def _should_disallow_pickle(self, disallow_pickle): # `disallow_pickle` set by function parameter, has priority # Otherwise fallback to library option `strict_mode` diff --git a/python/tests/conftest.py b/python/tests/conftest.py index 5b339191fa4..f02ad721a75 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -134,6 +134,21 @@ def _set_multiprocessing_start_method_for_macos(): pass +# ArcticDB refuses to unpickle stored symbols/metadata on read by default (RCE hardening). +# The test suite writes and reads back its own trusted pickled data, so enable pickle reads +# for all tests. Default-off behaviour and the opt-in are covered explicitly in +# tests/unit/arcticdb/version_store/test_normalization.py. +@pytest.fixture(autouse=True) +def _allow_pickle_reads_in_tests(): + from arcticdb.version_store import _normalization + + _normalization.set_allow_pickle_reads(True) + try: + yield + finally: + _normalization.set_allow_pickle_reads(None) + + # silence warnings about custom markers def pytest_configure(config): config.addinivalue_line("markers", "storage: Mark tests related to storage functionality") diff --git a/python/tests/unit/arcticdb/version_store/test_normalization.py b/python/tests/unit/arcticdb/version_store/test_normalization.py index 2b7bde3ca55..32a8a2e6b92 100644 --- a/python/tests/unit/arcticdb/version_store/test_normalization.py +++ b/python/tests/unit/arcticdb/version_store/test_normalization.py @@ -46,7 +46,11 @@ DataFrameNormalizer, NdArrayNormalizer, NPDDataFrame, + set_allow_pickle_reads, + allow_pickle_reads, + _ALLOW_PICKLE_READS_ENV_VAR, ) +from arcticdb.exceptions import UnsafePickleReadError from arcticdb.version_store._common import TimeFrame from arcticdb.util.test import ( CustomThing, @@ -425,6 +429,79 @@ def test_msg_pack_normalizer_strict_mode(): assert not norm.strict_mode +@pytest.fixture +def _pickle_reads_reset(): + # The suite-wide autouse fixture enables pickle reads; reset to the real default (defer to + # env var, which is unset) so these tests exercise the genuine default-off behaviour. + set_allow_pickle_reads(None) + try: + yield + finally: + set_allow_pickle_reads(True) + + +def _pack_pickle_payload(): + # Packing (write) is gated by strict_mode/disallow_pickle only, both off here, so this + # pickles regardless of the read-side allow_pickle_reads flag. + return MsgPackNormalizer()._msgpack_packb(errors.BoundaryError("bananas")) + + +def test_read_pickle_disabled_by_default(monkeypatch, _pickle_reads_reset): + monkeypatch.delenv(_ALLOW_PICKLE_READS_ENV_VAR, raising=False) + assert not allow_pickle_reads() + packed = _pack_pickle_payload() + norm = MsgPackNormalizer() + with pytest.raises(UnsafePickleReadError): + norm._msgpack_unpackb(packed) + + +def test_read_pickle_global_opt_in(_pickle_reads_reset): + packed = _pack_pickle_payload() + norm = MsgPackNormalizer() + set_allow_pickle_reads(True) + data = norm._msgpack_unpackb(packed) + assert isinstance(data, errors.BoundaryError) + assert data.args[0] == "bananas" + + +def test_read_pickle_instance_opt_in_overrides_global(_pickle_reads_reset): + packed = _pack_pickle_payload() + set_allow_pickle_reads(False) + norm = MsgPackNormalizer() + norm.allow_pickle_reads = True + data = norm._msgpack_unpackb(packed) + assert isinstance(data, errors.BoundaryError) + + +def test_read_pickle_env_var_opt_in(monkeypatch, _pickle_reads_reset): + packed = _pack_pickle_payload() + norm = MsgPackNormalizer() + monkeypatch.setenv(_ALLOW_PICKLE_READS_ENV_VAR, "1") + assert allow_pickle_reads() + assert isinstance(norm._msgpack_unpackb(packed), errors.BoundaryError) + monkeypatch.setenv(_ALLOW_PICKLE_READS_ENV_VAR, "0") + assert not allow_pickle_reads() + with pytest.raises(UnsafePickleReadError): + norm._msgpack_unpackb(packed) + + +def test_non_pickle_read_allowed_when_pickle_disabled(monkeypatch, _pickle_reads_reset): + monkeypatch.delenv(_ALLOW_PICKLE_READS_ENV_VAR, raising=False) + assert not allow_pickle_reads() + norm = MsgPackNormalizer() + packed = norm._msgpack_packb({"a": "1", "b": 2, "c": 3.0}) + assert norm._msgpack_unpackb(packed) == {"a": "1", "b": 2, "c": 3.0} + + +def test_user_metadata_pickle_disabled_by_default(monkeypatch, _pickle_reads_reset): + monkeypatch.delenv(_ALLOW_PICKLE_READS_ENV_VAR, raising=False) + udm = normalize_metadata({"obj": errors.BoundaryError("bananas")}) + with pytest.raises(UnsafePickleReadError): + denormalize_user_metadata(udm) + set_allow_pickle_reads(True) + assert denormalize_user_metadata(udm)["obj"].args[0] == "bananas" + + NT = namedtuple("NT", ["X", "Y"])