feat: require opt-in to read data encoded with the pickle codec

changes/4404.feature.md

@@ -0,0 +1 @@
+Reading data encoded with the `pickle` codec now requires an explicit opt-in, because unpickling data from an untrusted store can execute arbitrary code ([GHSA-jqc8-qjfv-5m6r](https://github.com/zarr-developers/numcodecs/security/advisories/GHSA-jqc8-qjfv-5m6r)). Enable it with `zarr.config.set({"array.allow_unsafe_pickle_decode": True})` or the environment variable `ZARR_ARRAY__ALLOW_UNSAFE_PICKLE_DECODE=True`. Without the opt-in, reading such data raises `zarr.errors.PickleCodecDisabledError`. numcodecs has its own opt-in (`numcodecs.pickles.ALLOW_UNSAFE_PICKLE_DECODE`), which must also be enabled. Writing with the `pickle` codec is unchanged.

changes/4539.bugfix.md

@@ -0,0 +1 @@
+The minimum supported numcodecs version is now 0.16.6, and 0.17.0 is excluded. Those versions leave `Pickle.decode` enabled by default, which allows arbitrary code execution when reading an untrusted Zarr v2 array ([GHSA-jqc8-qjfv-5m6r](https://github.com/zarr-developers/numcodecs/security/advisories/GHSA-jqc8-qjfv-5m6r)).

docs/user-guide/config.md

@@ -40,6 +40,7 @@ Configuration options include the following:
 - Whether empty chunks are written to storage `array.write_empty_chunks`
 - Enable experimental rectilinear chunks `array.rectilinear_chunks`
 - Whether missing chunks are filled with the array's fill value on read `array.read_missing_chunks` (default `True`). Set to `False` to raise a [`ChunkNotFoundError`][zarr.errors.ChunkNotFoundError] instead.
+- Whether data encoded with the `pickle` codec can be read `array.allow_unsafe_pickle_decode` (default `False`). Unpickling can execute arbitrary code, so only enable this for data from a source you trust, e.g. with `zarr.config.set({'array.allow_unsafe_pickle_decode': True})` or the environment variable `ZARR_ARRAY__ALLOW_UNSAFE_PICKLE_DECODE=True`. While it is disabled, reading such data raises a [`PickleCodecDisabledError`][zarr.errors.PickleCodecDisabledError]. Writing with the `pickle` codec is not affected. numcodecs has a separate opt-in for the same reason (`numcodecs.pickles.ALLOW_UNSAFE_PICKLE_DECODE = True`, or `NUMCODECS_ALLOW_UNSAFE_PICKLE_DECODE=1` before importing numcodecs); both must be enabled to read pickled data.
 - Async and threading options, e.g. `async.concurrency` and `threading.max_workers`
 - Selections of implementations of codecs, codec pipelines and buffers
 - Enabling GPU support with `zarr.config.enable_gpu()`. See [GPU support](gpu.md) for more.

pyproject.toml

@@ -47,7 +47,8 @@ requires-python = ">=3.12"
 dependencies = [
     'packaging>=22.0',
     'numpy>=2',
-    'numcodecs>=0.16',
+    # 0.16.6 and 0.17.1 refuse to unpickle untrusted data (GHSA-jqc8-qjfv-5m6r); 0.17.0 does not.
+    'numcodecs>=0.16.6,!=0.17.0',
     'google-crc32c>=1.5',
     'typing_extensions>=4.14',
     'donfig>=0.8',

src/zarr/codecs/_v2.py

@@ -8,6 +8,8 @@
 from numcodecs.compat import ensure_bytes, ensure_ndarray_like
 
 from zarr.abc.codec import ArrayBytesCodec
+from zarr.core.config import config
+from zarr.errors import PickleCodecDisabledError
 from zarr.registry import get_ndbuffer_class
 
 if TYPE_CHECKING:
@@ -16,6 +18,29 @@
     from zarr.core.buffer import Buffer, NDBuffer
 
 
+def _check_pickle_allowed(codecs: tuple[Numcodec | None, ...]) -> None:
+    """
+    Raise if any of ``codecs`` is the pickle codec and reading pickled data has not been enabled.
+
+    Unpickling can run arbitrary code, so data from an untrusted store must not be decoded
+    with the pickle codec unless the user has explicitly opted in.
+    """
+    if not any(getattr(codec, "codec_id", None) == "pickle" for codec in codecs):
+        return
+    # Compare with ``True`` rather than testing truthiness: donfig keeps an environment
+    # variable it cannot parse as a Python literal (e.g. ``false``) as a non-empty string.
+    if config.get("array.allow_unsafe_pickle_decode") is not True:
+        raise PickleCodecDisabledError(
+            "Refusing to decode data with the 'pickle' codec, because unpickling data "
+            "from an untrusted source can execute arbitrary code. If you trust this data, "
+            "enable the pickle codec with zarr.config.set({'array.allow_unsafe_pickle_decode': True}) "
+            "or set the environment variable ZARR_ARRAY__ALLOW_UNSAFE_PICKLE_DECODE=True. numcodecs has "
+            "its own opt-in, which must be enabled as well: set "
+            "numcodecs.pickles.ALLOW_UNSAFE_PICKLE_DECODE = True, or set the environment "
+            "variable NUMCODECS_ALLOW_UNSAFE_PICKLE_DECODE=1 before importing numcodecs."
+        )
+
+
 @dataclass(frozen=True)
 class V2Codec(ArrayBytesCodec):
     filters: tuple[Numcodec, ...] | None
@@ -28,6 +53,7 @@ def _decode_sync(
         chunk_bytes: Buffer,
         chunk_spec: ArraySpec,
     ) -> NDBuffer:
+        _check_pickle_allowed((self.compressor, *(self.filters or ())))
         cdata = chunk_bytes.as_array_like()
         # decompress
         if self.compressor:

src/zarr/core/config.py

@@ -103,6 +103,9 @@ def enable_gpu(self) -> ConfigSet:
                 "rectilinear_chunks": False,
                 "sharding_coalesce_max_gap_bytes": 1 << 20,  # 1 MiB
                 "sharding_coalesce_max_bytes": 16 << 20,  # 16 MiB
+                # Decoding data with the pickle codec can execute arbitrary code,
+                # so it must be explicitly enabled.
+                "allow_unsafe_pickle_decode": False,
             },
             "async": {"concurrency": 10, "timeout": None},
             "threading": {"max_workers": None},

src/zarr/errors.py

@@ -12,6 +12,7 @@
     "MetadataValidationError",
     "NegativeStepError",
     "NodeTypeValidationError",
+    "PickleCodecDisabledError",
     "UnknownCodecError",
     "UnstableSpecificationWarning",
     "VindexInvalidSelectionError",
@@ -101,6 +102,15 @@ class UnknownCodecError(BaseZarrError):
     """
 
 
+class PickleCodecDisabledError(BaseZarrError):
+    """
+    Raised when reading data encoded with the pickle codec without opting in.
+
+    Unpickling can execute arbitrary code, so decoding with the pickle codec is
+    disabled unless the ``array.allow_unsafe_pickle_decode`` configuration option is set to ``True``.
+    """
+
+
 class NodeTypeValidationError(MetadataValidationError):
     """
     Specialized exception when the node_type of the metadata document is incorrect.

tests/test_config.py

@@ -59,6 +59,7 @@ def test_config_defaults_set() -> None:
                     "rectilinear_chunks": False,
                     "sharding_coalesce_max_gap_bytes": 1 << 20,
                     "sharding_coalesce_max_bytes": 16 << 20,
+                    "allow_unsafe_pickle_decode": False,
                 },
                 "async": {"concurrency": 10, "timeout": None},
                 "threading": {"max_workers": None},

tests/test_v2.py

@@ -1,10 +1,13 @@
 import json
+import pickle
 from pathlib import Path
 from typing import Any, Literal
 
+import numcodecs.errors
+import numcodecs.pickles
 import numpy as np
 import pytest
-from numcodecs import Delta, Zlib
+from numcodecs import Delta, Pickle, VLenUTF8, Zlib
 from numcodecs.blosc import Blosc
 from numcodecs.zstd import Zstd
 
@@ -20,7 +23,7 @@
 from zarr.core.dtype.wrapper import ZDType
 from zarr.core.group import Group
 from zarr.core.sync import sync
-from zarr.errors import ZarrDeprecationWarning
+from zarr.errors import PickleCodecDisabledError, ZarrDeprecationWarning
 from zarr.storage import MemoryStore, StorePath
 
 
@@ -310,3 +313,116 @@ def test_other_dtype_roundtrip(fill_value: bytes | None, tmp_path: Path) -> None
     za[...] = a
     za = zarr.open_array(store=array_path)
     assert (a == za[:]).all()
+
+
+def _pickle_compressed_array() -> zarr.Array[Any]:
+    # Writing with the pickle codec is allowed without opting in, only decoding is gated.
+    arr = zarr.create_array(
+        store={},
+        shape=(3,),
+        dtype=VariableLengthUTF8(),  # type: ignore[arg-type]
+        zarr_format=2,
+        filters=[VLenUTF8()],
+        compressors=Pickle(),
+        fill_value="",
+    )
+    arr[:] = np.array(["a", "bb", "ccc"])
+    return arr
+
+
+@pytest.fixture
+def allow_numcodecs_unpickle(monkeypatch: pytest.MonkeyPatch) -> None:
+    """
+    Opt in to numcodecs' own gate on ``Pickle.decode``.
+
+    numcodecs refuses to unpickle unless the process opts in (GHSA-jqc8-qjfv-5m6r). zarr's
+    opt-in is separate, so tests that read pickled data after opting in to zarr also need this.
+    """
+    monkeypatch.setattr(numcodecs.pickles, "ALLOW_UNSAFE_PICKLE_DECODE", True)
+
+
+def test_pickle_codec_read_disabled_by_default() -> None:
+    """Reading data encoded with the pickle codec fails unless the user opts in."""
+    arr = _pickle_compressed_array()
+    with pytest.raises(
+        PickleCodecDisabledError, match=r"array\.allow_unsafe_pickle_decode"
+    ) as excinfo:
+        arr[:]
+    # numcodecs has its own opt-in; the message must name it so users are not left guessing
+    assert "ALLOW_UNSAFE_PICKLE_DECODE" in str(excinfo.value)
+
+
+def test_pickle_codec_read_does_not_unpickle() -> None:
+    """A malicious chunk is not unpickled when the pickle codec is disabled."""
+
+    class Payload:
+        def __reduce__(self) -> tuple[Any, ...]:
+            return (exec, ("raise AssertionError('payload was unpickled')",))
+
+    meta = {
+        "zarr_format": 2,
+        "shape": [1],
+        "chunks": [1],
+        "dtype": "|O",
+        "compressor": {"id": "pickle"},
+        "fill_value": None,
+        "filters": [{"id": "vlen-utf8"}],
+        "order": "C",
+    }
+    buffer_cls = default_buffer_prototype().buffer
+    store = {
+        ".zarray": buffer_cls.from_bytes(json.dumps(meta).encode()),
+        "0": buffer_cls.from_bytes(pickle.dumps(Payload())),
+    }
+    arr = zarr.open_array(store=MemoryStore(store), mode="r", zarr_format=2)
+    with pytest.raises(PickleCodecDisabledError):
+        arr[:]
+
+
+def test_pickle_codec_zarr_opt_in_does_not_bypass_numcodecs(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Opting in to zarr does not disable numcodecs' own gate on ``Pickle.decode``."""
+    monkeypatch.setattr(numcodecs.pickles, "ALLOW_UNSAFE_PICKLE_DECODE", False)
+    arr = _pickle_compressed_array()
+    with (
+        config.set({"array.allow_unsafe_pickle_decode": True}),
+        pytest.raises(numcodecs.errors.UnsafeDecodeError),
+    ):
+        arr[:]
+
+
+@pytest.mark.usefixtures("allow_numcodecs_unpickle")
+def test_pickle_codec_read_enabled_via_config() -> None:
+    arr = _pickle_compressed_array()
+    with config.set({"array.allow_unsafe_pickle_decode": True}):
+        np.testing.assert_array_equal(arr[:], np.array(["a", "bb", "ccc"]))
+
+
+@pytest.mark.usefixtures("allow_numcodecs_unpickle")
+@pytest.mark.parametrize(("env_value", "allowed"), [("True", True), ("false", False)])
+def test_pickle_codec_read_enabled_via_env_var(
+    monkeypatch: pytest.MonkeyPatch, env_value: str, allowed: bool
+) -> None:
+    """
+    The opt-in can be set with an environment variable. Only a value that parses to ``True``
+    enables it: donfig keeps ``false`` as a (truthy) string, which must not enable it.
+    """
+    arr = _pickle_compressed_array()
+    monkeypatch.setenv("ZARR_ARRAY__ALLOW_UNSAFE_PICKLE_DECODE", env_value)
+    config.refresh()
+    if allowed:
+        np.testing.assert_array_equal(arr[:], np.array(["a", "bb", "ccc"]))
+    else:
+        with pytest.raises(PickleCodecDisabledError):
+            arr[:]
+
+
+def test_pickle_opt_in_does_not_affect_other_codecs() -> None:
+    """Arrays that do not use the pickle codec are read without opting in."""
+    assert config.get("array.allow_unsafe_pickle_decode") is False
+    arr = zarr.create_array(
+        store={}, shape=(3,), dtype="<i4", zarr_format=2, filters=[Delta(dtype="<i4")]
+    )
+    arr[:] = [1, 2, 3]
+    np.testing.assert_array_equal(arr[:], [1, 2, 3])