diff --git a/monai/auto3dseg/utils.py b/monai/auto3dseg/utils.py index 518c91919d..1bcae85d83 100644 --- a/monai/auto3dseg/utils.py +++ b/monai/auto3dseg/utils.py @@ -20,6 +20,7 @@ import warnings from copy import deepcopy from numbers import Number +from pydoc import locate from typing import Any, cast import numpy as np @@ -62,6 +63,42 @@ def _require_pickle_allowed() -> None: raise RuntimeError(_PICKLE_DISABLED_MSG) +def _reject_non_algo_target(target: object, filename: str) -> None: + """ + Require that ``target`` names an ``Algo`` subclass before it is instantiated. + + ``algo_from_json`` resolves ``_target_`` through ``ConfigParser``, which imports the dotted path + and calls it. ``algo_object.json`` has exactly one legitimate target type -- an ``Algo`` + subclass -- so resolving the name and checking it here rejects payloads such as + ``subprocess.call`` or ``os.system`` before they are ever invoked. + + Importing the named module still runs that module's top-level code, so this narrows the sink + rather than closing it: an untrusted ``algo_object.json`` combined with an attacker-controlled + template directory remains dangerous. + + Args: + target: the ``_target_`` value read from the file. + filename: the file the value came from, used in the error message. + + Raises: + ValueError: if ``target`` is not a string, or does not resolve to a concrete ``Algo`` subclass. + ModuleNotFoundError: if the module cannot be imported, so callers can try the next + template path. + """ + if not isinstance(target, str): + raise ValueError(f"invalid `_target_` in {filename}: expected a string, got {type(target).__name__}.") + resolved = locate(target) + if resolved is None: + raise ModuleNotFoundError(f"cannot resolve `_target_` {target!r} from {filename}.") + if not (isinstance(resolved, type) and resolved is not Algo and issubclass(resolved, Algo)): + raise ValueError( + f"refusing to instantiate `_target_` {target!r} from {filename}: it resolves to " + f"{resolved!r}, which is not a subclass of monai.auto3dseg.Algo. Only Algo subclasses " + "may be named in an algo_object.json " + "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv)." + ) + + measure_np, has_measure = optional_import("skimage.measure", "0.14.2", min_version) cp, has_cp = optional_import("cupy") @@ -493,16 +530,16 @@ def algo_from_json(filename: str, template_path: PathLike | None = None, **kwarg if state_template_path: algo_config["template_path"] = state_template_path - warnings.warn( - f"Loading {filename}: the file's `_target_` value is resolved to an imported callable and " - "invoked, and template directories from the file may be added to `sys.path`; only load " - "algo_object.json files from a source you trust " - "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv).", - stacklevel=2, - ) + _reject_non_algo_target(target, filename) parser = ConfigParser(algo_config) algo = parser.get_parsed_content() + if not isinstance(algo, Algo): + raise ValueError( + f"refusing to return the object built from '{target}' in {filename}: it is a " + f"{type(algo).__name__}, not an Algo instance " + "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv)." + ) used_template_path = path break except ModuleNotFoundError as e: diff --git a/tests/apps/test_auto3dseg.py b/tests/apps/test_auto3dseg.py index c310afc76a..5cf2f0fc77 100644 --- a/tests/apps/test_auto3dseg.py +++ b/tests/apps/test_auto3dseg.py @@ -15,7 +15,6 @@ import os import tempfile import unittest -import warnings from copy import deepcopy from numbers import Number @@ -637,22 +636,20 @@ def tearDown(self) -> None: class TestAlgoFromJsonSecurityWarning(unittest.TestCase): - def test_warns_about_untrusted_target(self) -> None: + def test_rejects_untrusted_target(self) -> None: + """Verify that a ``_target_`` pointing to a non-``Algo`` class is rejected. + + ``_DummyAlgo`` is a plain class (not an ``Algo`` subclass); loading it must raise + ``ValueError`` rather than instantiate the class, per GHSA-2wx3. + """ with tempfile.TemporaryDirectory() as tmpdir: algo_file = os.path.join(tmpdir, "algo_object.json") with open(algo_file, "w", encoding="utf-8") as f: json.dump({"_target_": f"{__name__}._DummyAlgo"}, f) - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") + with self.assertRaisesRegex(ValueError, "refusing to instantiate"): algo_from_json(algo_file) - messages = [str(w.message) for w in caught] - self.assertTrue( - any("algo_object.json" in msg and "trust" in msg for msg in messages), - f"Keywords 'algo_object.json' and 'trust' not found in warning messages: {messages}", - ) - if __name__ == "__main__": unittest.main() diff --git a/tests/auto3dseg/test_algo_target_allowlist.py b/tests/auto3dseg/test_algo_target_allowlist.py new file mode 100644 index 0000000000..6c7b98b6fd --- /dev/null +++ b/tests/auto3dseg/test_algo_target_allowlist.py @@ -0,0 +1,75 @@ +# Copyright (c) MONAI Consortium +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations + +import json +import os +import tempfile +import unittest + +from monai.auto3dseg.utils import _reject_non_algo_target, algo_from_json + + +class TestAlgoTargetAllowlist(unittest.TestCase): + """Regression tests for GHSA-2wx3-8x3w-r8qv. + + ``algo_from_json`` resolves the file's ``_target_`` through ``ConfigParser``, which imports the + dotted path and calls it. ``algo_object.json`` has exactly one legitimate target type, so a + ``_target_`` that is not an ``Algo`` subclass is rejected before it is instantiated. + """ + + def test_code_execution_targets_are_rejected(self): + for target in ("subprocess.call", "os.system", "builtins.eval", "builtins.exec", "shutil.rmtree"): + with self.subTest(target=target): + with self.assertRaises(ValueError) as ctx: + _reject_non_algo_target(target, "algo_object.json") + self.assertIn("GHSA-2wx3-8x3w-r8qv", str(ctx.exception)) + + def test_non_class_target_is_rejected(self): + """A module or plain function is not an Algo subclass.""" + with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"): + _reject_non_algo_target("json.dumps", "algo_object.json") + + def test_unresolvable_target_raises_module_not_found(self): + """Unresolvable names raise ModuleNotFoundError so the caller can try the next path.""" + with self.assertRaises(ModuleNotFoundError): + _reject_non_algo_target("no_such_module.NoSuchAlgo", "algo_object.json") + + def test_algo_subclass_is_accepted(self): + """A real Algo subclass passes the check.""" + _reject_non_algo_target("monai.apps.auto3dseg.BundleAlgo", "algo_object.json") + + def test_non_string_target_is_rejected(self): + """A JSON ``null`` ``_target_`` raises the documented ``ValueError``, not ``AttributeError``.""" + with self.assertRaisesRegex(ValueError, r"expected a string"): + _reject_non_algo_target(None, "algo_object.json") + + def test_bare_algo_base_class_is_rejected(self): + """The abstract ``Algo`` base class is not itself a valid instantiation target.""" + with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"): + _reject_non_algo_target("monai.auto3dseg.algo_gen.Algo", "algo_object.json") + + def test_algo_from_json_rejects_payload_target(self): + """End to end: a malicious algo_object.json never reaches instantiation.""" + with tempfile.TemporaryDirectory() as tempdir: + path = os.path.join(tempdir, "algo_object.json") + marker = os.path.join(tempdir, "PWNED") + with open(path, "w") as f: + json.dump({"_target_": "subprocess.call", "args": ["/bin/sh", "-c", f"touch {marker}"]}, f) + + with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"): + algo_from_json(path) + self.assertFalse(os.path.exists(marker), "the algo_object.json payload executed") + + +if __name__ == "__main__": + unittest.main()