diff --git a/haystack/core/serialization_security.py b/haystack/core/serialization_security.py index 4910b21973f..61591ba4b8a 100644 --- a/haystack/core/serialization_security.py +++ b/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..`` 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. diff --git a/haystack/utils/callable_serialization.py b/haystack/utils/callable_serialization.py index beaf38c9bfd..ca7ba8758e0 100644 --- a/haystack/utils/callable_serialization.py +++ b/haystack/utils/callable_serialization.py @@ -4,6 +4,7 @@ import inspect from collections.abc import Callable +from types import ModuleType from typing import Any from haystack import logging @@ -11,6 +12,7 @@ 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.") diff --git a/haystack/utils/type_serialization.py b/haystack/utils/type_serialization.py index 8ef22371be4..9ee63746be7 100644 --- a/haystack/utils/type_serialization.py +++ b/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 diff --git a/releasenotes/notes/fix-deserialize-callable-allowlist-bypass-0d99a89c1a3b4a9f.yaml b/releasenotes/notes/fix-deserialize-callable-allowlist-bypass-0d99a89c1a3b4a9f.yaml new file mode 100644 index 00000000000..038222b5002 --- /dev/null +++ b/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. diff --git a/test/core/test_serialization_security.py b/test/core/test_serialization_security.py index 809ad53112e..647ced22a72 100644 --- a/test/core/test_serialization_security.py +++ b/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(): """