fix: strengthen deserialization allowlist checks to prevent bypass vulnerabilities (#12028)

Co-authored-by: David S. Batista <dsbatista@gmail.com>

Author: bogdankostic

haystack/core/serialization_security.py

@@ -22,6 +22,7 @@
 from collections.abc import Iterable, Iterator
 from contextlib import contextmanager
 from dataclasses import dataclass, field
+from types import ModuleType
 
 from haystack.core.errors import DeserializationError
 
@@ -161,6 +162,65 @@ def _check_module_allowed(module_name: str) -> None:
     )
 
 
+def _check_resolved_module_allowed(resolved: object, declared_module: str | None = None) -> None:
+    """
+    Gate on the module a resolved object *actually* comes from, not on the declared handle.
+
+    The allowlist checks that run earlier in deserialization apply to the declared dotted path,
+    which an attacker controls. A crafted handle can name an object re-exported as an attribute of
+    an allowlisted module and thereby escape the allowlist:
+
+    - a module: ``haystack.utils.auth.os`` resolves to the standard-library ``os`` module, because
+      ``haystack.utils.auth`` does ``import os`` at module scope. The handle has the allowlisted
+      ``haystack`` prefix, but the resolved object belongs to the un-allowlisted ``os`` module.
+    - a class or function: ``haystack.<mod>.<Name>`` could resolve a class/function imported into
+      an allowlisted module from an un-allowlisted one (e.g. a re-exported ``subprocess.Popen``).
+
+    Checking the resolved object's real module closes both gaps. Objects without a usable
+    ``__module__`` (e.g. ``functools.partial`` instances) are left to the other gates; they are not
+    reachable as gadgets through an attribute walk that stays entirely inside allowlisted modules.
+
+    :param resolved:
+        The object resolved from the serialized handle.
+    :param declared_module:
+        The allowlisted module the object was actually resolved from (the module that was imported
+        and walked). Used to accept the private implementation module that backs a public module —
+        see below.
+    :raises DeserializationError:
+        If the resolved object's real module is not on the allowlist.
+    """
+    if _get_context().unsafe:
+        return
+    # Builtins are gated separately and authoritatively — by the identity denylist
+    # (`_check_not_denied_builtin`) in the callable path and by the type requirement
+    # (`_check_builtin_is_type`) in the class path. They can also report a private `__module__`
+    # (e.g. `open.__module__ == "_io"`), so exempt any object that is a genuine builtin (reachable
+    # under its own name in the `builtins` module) from the module check to avoid shadowing those
+    # gates. This exemption cannot widen access: a dangerous builtin is still stopped downstream.
+    name = getattr(resolved, "__name__", None)
+    if isinstance(name, str) and getattr(builtins, name, None) is resolved:
+        return
+    # Modules carry their identity in `__name__`, not `__module__`.
+    module_name = resolved.__name__ if isinstance(resolved, ModuleType) else getattr(resolved, "__module__", None)
+    if not (isinstance(module_name, str) and module_name):
+        return
+    if _is_module_allowed(module_name):
+        return
+    # Private implementation module backing an allowlisted public module: a symbol legitimately
+    # exposed by an allowlisted module can report a private `__module__` — e.g. `operator.add`
+    # resolves to `_operator.add`, `io.StringIO` to `_io.StringIO`. Accept it only when the private
+    # module is the C accelerator of the *same* allowlisted module the object was resolved from
+    # (`_operator` backs `operator`, `_io` backs `io`). This never admits a public, un-allowlisted
+    # module such as `os` or `subprocess`, so the escape gadgets stay blocked.
+    if (
+        declared_module is not None
+        and module_name.lstrip("_") == declared_module
+        and _is_module_allowed(declared_module)
+    ):
+        return
+    _check_module_allowed(module_name)
+
+
 def _is_denied_builtin(resolved: object) -> bool:
     """
     Return whether `resolved` is one of the builtins denied for callable deserialization.

haystack/utils/callable_serialization.py

@@ -4,13 +4,15 @@
 
 import inspect
 from collections.abc import Callable
+from types import ModuleType
 from typing import Any
 
 from haystack import logging
 from haystack.core.errors import DeserializationError, SerializationError
 from haystack.core.serialization_security import (
     _check_module_allowed,
     _check_not_denied_builtin,
+    _check_resolved_module_allowed,
     _is_denied_builtin,
     _is_module_allowed,
 )
@@ -84,13 +86,14 @@ def deserialize_callable(callable_handle: str) -> Callable:
 
     parts = callable_handle.split(".")
 
-    # Allow if any prefix is on the allowlist; checking each one individually would wrongly
-    # reject patterns like `j*on` against `json.dumps` (matches `json`, not the full handle).
-    if not any(_is_module_allowed(".".join(parts[:i])) for i in range(1, len(parts) + 1)):
-        _check_module_allowed(callable_handle)  # raises with the standard help message
-
     for i in range(len(parts), 0, -1):
         module_name = ".".join(parts[:i])
+        # Only import modules that are on the allowlist. Gating the import (rather than a mere
+        # string prefix of the handle) means a disallowed module is never imported for its
+        # side effects, and the resolver can only ever start from a trusted module. Shorter
+        # prefixes are tried in turn, so `json.dumps` still resolves when `json` is allowed.
+        if not _is_module_allowed(module_name):
+            continue
         try:
             mod: Any = thread_safe_import(module_name)
         except Exception:
@@ -104,6 +107,12 @@ def deserialize_callable(callable_handle: str) -> Callable:
             except AttributeError as e:
                 container = getattr(attr_value, "__name__", type(attr_value).__name__)
                 raise DeserializationError(f"Could not find attribute '{part}' in {container}") from e
+            # A crafted handle can walk into a *module* re-exported as an attribute of an
+            # allowlisted module (e.g. `haystack.utils.auth.os` -> the `os` module). The declared
+            # path had an allowlisted prefix, but the module's real identity (`__name__`) is not
+            # allowlisted. Re-check every module hop so the walk cannot escape the allowlist.
+            if isinstance(attr_value, ModuleType):
+                _check_module_allowed(attr_value.__name__)
 
         # when the attribute is a classmethod, we need the underlying function
         if isinstance(attr_value, (classmethod, staticmethod)):
@@ -120,11 +129,20 @@ def deserialize_callable(callable_handle: str) -> Callable:
         if not callable(attr_value):
             raise DeserializationError(f"The final attribute is not callable: {attr_value}")
 
+        # Final defense: gate on the module the resolved callable actually comes from, not on the
+        # declared handle. This catches a dangerous callable bound as a plain (non-module) attribute
+        # of an allowlisted object, which the module-walk check above would not see. `module_name`
+        # is the allowlisted module we resolved from, so a private C accelerator backing it (e.g.
+        # `operator.add` -> `_operator`) is still accepted.
+        _check_resolved_module_allowed(attr_value, declared_module=module_name)
+
         # `builtins` is on the allowlist (for `builtins.print` etc.), so the module check
         # above does not stop dangerous builtins like `eval`/`exec` from resolving here. Block them.
         _check_not_denied_builtin(attr_value, callable_handle)
 
         return attr_value
 
-    # Fallback if we never find anything
+    # Nothing on the allowlist was importable. Surface the standard allowlist error when the
+    # top-level module is untrusted; otherwise report a plain resolution failure.
+    _check_module_allowed(callable_handle)
     raise DeserializationError(f"Could not import '{callable_handle}' as a module or callable.")

haystack/utils/type_serialization.py

@@ -12,7 +12,11 @@
 
 from haystack import logging
 from haystack.core.errors import DeserializationError
-from haystack.core.serialization_security import _check_builtin_is_type, _check_module_allowed
+from haystack.core.serialization_security import (
+    _check_builtin_is_type,
+    _check_module_allowed,
+    _check_resolved_module_allowed,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -256,6 +260,13 @@ def _import_class_by_name(fully_qualified_name: str) -> Any:
         )
         module = thread_safe_import(module_path)
         resolved = getattr(module, attr_name)
+        # `_check_module_allowed` above gates the declared module path, which the caller controls.
+        # A class/module re-exported as an attribute of an allowlisted module (e.g.
+        # `haystack.utils.auth.os` -> the `os` module, or a re-exported `subprocess.Popen`) would
+        # otherwise slip through. Re-check the module the object actually belongs to; `module_path`
+        # is the allowlisted module it was resolved from, so a private C accelerator backing it
+        # (e.g. `io.StringIO` -> `_io`) is still accepted.
+        _check_resolved_module_allowed(resolved, declared_module=module_path)
         if module_path == "builtins":
             _check_builtin_is_type(resolved, fully_qualified_name)
         return resolved

releasenotes/notes/fix-deserialize-callable-allowlist-bypass-0d99a89c1a3b4a9f.yaml

@@ -0,0 +1,14 @@
+---
+security:
+  - |
+    Fixed a deserialization-allowlist bypass that could allow arbitrary code execution when
+    loading an untrusted pipeline (``Pipeline.load`` / ``Pipeline.loads`` / ``Pipeline.from_dict``).
+    The module allowlist was enforced against a string prefix of the serialized handle, while the
+    callable/class resolvers walked attributes from the deepest importable module. Because an
+    allowlisted package can expose another module as an attribute (e.g. a module-scope ``import os``
+    makes ``haystack.utils.auth.os`` the standard-library ``os`` module), a handle such as
+    ``haystack.utils.auth.os.system`` passed the allowlist and resolved to ``os.system``. The
+    allowlist is now enforced against the module each object is *actually* resolved from — every
+    module hop during attribute resolution is re-checked, and the resolved callable's/class's real
+    module must be on the allowlist. Legitimate handles (including harmless builtins such as
+    ``builtins.print``) continue to resolve.

test/core/test_serialization_security.py

@@ -2,8 +2,10 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+import functools
 import io
 import json
+import operator
 import subprocess
 from collections.abc import Callable
 
@@ -302,6 +304,108 @@ def test_unsafe_mode_bypasses_the_block(self):
             assert deserialize_callable("builtins.eval") is eval
 
 
+class TestModuleAttributeWalkBypass:
+    """
+    The allowlist must be enforced against the module a handle
+    *actually resolves to*, not against a string prefix of the declared handle.
+
+    An allowlisted package can expose another module as an attribute (a module-scope `import os`
+    makes `haystack.utils.auth.os` the standard-library `os` module). A handle like
+    `haystack.utils.auth.os.system` carries the allowlisted `haystack` prefix, so the old
+    prefix-only check accepted it, and the resolver then walked `.os.system` into the un-allowlisted
+    `os` module — a deserialization-allowlist bypass reaching arbitrary command execution.
+    """
+
+    # (handle, module the walk escapes into) — real gadgets present in the default install.
+    _GADGETS = [
+        ("haystack.utils.auth.os.system", "os"),
+        ("haystack.utils.auth.os.popen", "os"),
+        ("haystack.components.converters.output_adapter.ast.literal_eval", "ast"),
+    ]
+
+    @pytest.mark.parametrize("handle, escaped_module", _GADGETS)
+    def test_callable_walk_into_unallowlisted_module_rejected(self, handle, escaped_module):
+        with pytest.raises(DeserializationError, match=f"module '{escaped_module}'"):
+            deserialize_callable(handle)
+
+    def test_class_path_module_leak_rejected(self):
+        # The class path resolves `haystack.utils.auth.os` to the `os` module itself; it must not
+        # leak an un-allowlisted module as if it were a class.
+        with pytest.raises(DeserializationError, match="module 'os'"):
+            import_class_by_name("haystack.utils.auth.os")
+
+    def test_type_path_module_leak_rejected(self):
+        with pytest.raises(DeserializationError, match="module 'os'"):
+            deserialize_type("haystack.utils.auth.os")
+
+    def test_resolved_module_check_covers_plain_attribute(self, monkeypatch):
+        # Defense-in-depth: a dangerous callable bound as a plain (non-module) attribute of an
+        # allowlisted module is not caught by the module-walk check but must still be rejected by
+        # the resolved-`__module__` gate (`subprocess.getoutput.__module__ == "subprocess"`).
+        monkeypatch.setattr("haystack.utils.auth.injected_gadget", subprocess.getoutput, raising=False)
+        with pytest.raises(DeserializationError, match="module 'subprocess'"):
+            deserialize_callable("haystack.utils.auth.injected_gadget")
+
+    def test_legitimate_haystack_callable_still_resolves(self):
+        from haystack.utils.callable_serialization import serialize_callable
+
+        handle = "haystack.utils.callable_serialization.serialize_callable"
+        assert deserialize_callable(handle) is serialize_callable
+
+    def test_pipeline_loads_rejects_gadget_yaml(self):
+        # End-to-end through the public default-safe API: an attacker-supplied pipeline naming the
+        # gadget as an `OutputAdapter` custom filter must be rejected at load time.
+        gadget_yaml = (
+            "components:\n"
+            "  adapter:\n"
+            "    type: haystack.components.converters.output_adapter.OutputAdapter\n"
+            "    init_parameters:\n"
+            '      template: "{{ x }}"\n'
+            "      output_type: str\n"
+            "      custom_filters:\n"
+            '        pwn: "haystack.utils.auth.os.system"\n'
+            "connections: []\n"
+        )
+        with pytest.raises(DeserializationError):
+            Pipeline.loads(gadget_yaml)
+
+
+class TestPrivateBackingModuleCompatibility:
+    """
+    A symbol legitimately exposed by an allowlisted public module can report a *private* C
+    accelerator as its `__module__` (`operator.add.__module__ == "_operator"`,
+    `io.StringIO.__module__ == "_io"`). The resolved-module gate must still accept these when the
+    private module backs the allowlisted module the object was resolved from — otherwise allowing
+    a public module would silently fail to resolve its own members and force users to widen trust
+    to low-level private modules.
+    """
+
+    def test_callable_from_private_backing_module_resolves(self):
+        with _deserialization_context(allowed_modules=["operator"]):
+            assert deserialize_callable("operator.add") is operator.add
+
+    def test_callable_functools_reduce_resolves(self):
+        with _deserialization_context(allowed_modules=["functools"]):
+            assert deserialize_callable("functools.reduce") is functools.reduce
+
+    def test_type_from_private_backing_module_resolves(self):
+        with _deserialization_context(allowed_modules=["io"]):
+            assert deserialize_type("io.StringIO") is io.StringIO
+
+    def test_import_class_from_private_backing_module_resolves(self):
+        with _deserialization_context(allowed_modules=["io"]):
+            assert import_class_by_name("io.StringIO") is io.StringIO
+
+    def test_exemption_is_scoped_to_the_matching_private_module(self, monkeypatch):
+        # The exemption only trusts the private module that backs the *same* allowlisted module.
+        # A symbol whose real module is a different (still un-allowlisted) private module must stay
+        # blocked: here `operator` is allowed but the injected attribute resolves to `_io`.
+        monkeypatch.setattr("operator.injected_gadget", io.StringIO, raising=False)
+        with _deserialization_context(allowed_modules=["operator"]):
+            with pytest.raises(DeserializationError, match="module '_io'"):
+                deserialize_callable("operator.injected_gadget")
+
+
 @pytest.fixture
 def _registered_untrusted_component():
     """